[PATCH] D68683: ARM] Fix arm_neon.h with -flax-vector-conversions=none

[Sjoerd Meijer via Phabricator via cfe-commits]

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

Nice one, thanks for fixing! I didn't have the bandwidth to look into this.


Repository:
  rC Clang

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

https://reviews.llvm.org/D68683



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

[PATCH] D68682: Clang-tidy fixes should avoid creating new blank lines

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Sorry, I'm going to give a drive-by comment (which you can ignore), mainly by 
because you mentioned `clang-format`.

This seems like a good idea as obviously it solves this problem, however, isn't 
rather like trying to fix it after the fact?

what if (I'm sure there isn't anything currently doing this so maybe a moot 
point), someone wrote a clang-tidy checker to say insert newlines between 
things, sort of imagine something like 
https://bugs.llvm.org/show_bug.cgi?id=42767 where the person wanted to add 
missing newlines based on some semantic rule rather than the more traditional 
way of handling it in clang-format.

This code change kind of says doing anything like replacing something with a 
newline would be stripped away, to me it feels like this removal of extra white 
space needs to be handled at the point the replacement is created and not on 
all the final replacements where the context is lost. (perhaps you already 
considered that)

Just my 2c worth. but I do think its good to remove the newlines in this case 
so thank you for that.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68682



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

[PATCH] D68682: Clang-tidy fixes should avoid creating new blank lines

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added a reviewer: gribozavr.
gribozavr requested changes to this revision.
gribozavr added a comment.
This revision now requires changes to proceed.

+1 to what MyDeveloperDay said. The infrastructure can't know whether the 
newlines are intentional or not. Checks that edit the source code should be 
improved to delete newlines where they become unnecessary. I don't think we can 
accept the patch that changes how we apply edits.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68682



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

[PATCH] D67536: [WIP] [clangd] Add support for an inactive regions notification

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D67536#1700872 , @nridge wrote:

> > It also lets them consistently highlight part of the line (e.g. dead 
> > expressions or statements can be marked in gray even if they are on the 
> > same line).
>
> Highlighting part of a line is not applicable to inactive preprocessor 
> branches in C++.


Note that the opposite is true - inactive preprocessor branches can be 
expressed as range-based highlightings.

> If we later introduce highlightings for dead expressions or statements, we 
> can of course use character ranges and not lines for them.

Then we'll have two modes of operation - line-based and range-based 
highlightings. Clients will have to support both, we'll have code dealing with 
both.
The alternative is having only range-based highlightings and using them 
everywhere. That's the main reason I would advocate for range-based.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67536



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

Re: r374006 - Reland 'Add VFS support for sanitizers' blacklist'

[Ilya Biryukov via cfe-commits]

Hi Jan,

This patch seems to assume VFS will only re-map some paths, but still point
into the physical filesystem.
This may not be the case, e.g. in unit tests.

This also manages to break some of our internal clang-tidy integrations for
obscure reasons.

Can we instead just pass VFS instance to the SanitizerBlacklist and use
relative paths?

We might also need to revert the patch to unbreak our release, sorry about
the inconvenience.

On Tue, Oct 8, 2019 at 3:11 AM Jan Korous via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: jkorous
> Date: Mon Oct  7 18:13:17 2019
> New Revision: 374006
>
> URL: http://llvm.org/viewvc/llvm-project?rev=374006&view=rev
> Log:
> Reland 'Add VFS support for sanitizers' blacklist'
>
> The original patch broke the test for Windows.
> Trying to fix as per Reid's suggestions outlined here:
> https://reviews.llvm.org/rC371663
>
> Differential Revision: https://reviews.llvm.org/D67742
>
> Added:
> cfe/trunk/test/CodeGen/Inputs/sanitizer-blacklist-vfsoverlay.yaml
> Modified:
> cfe/trunk/lib/AST/ASTContext.cpp
> cfe/trunk/test/CodeGen/ubsan-blacklist.c
>
> Modified: cfe/trunk/lib/AST/ASTContext.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=374006&r1=374005&r2=374006&view=diff
>
> ==
> --- cfe/trunk/lib/AST/ASTContext.cpp (original)
> +++ cfe/trunk/lib/AST/ASTContext.cpp Mon Oct  7 18:13:17 2019
> @@ -72,6 +72,7 @@
>  #include "llvm/ADT/PointerUnion.h"
>  #include "llvm/ADT/STLExtras.h"
>  #include "llvm/ADT/SmallPtrSet.h"
> +#include "llvm/ADT/SmallString.h"
>  #include "llvm/ADT/SmallVector.h"
>  #include "llvm/ADT/StringExtras.h"
>  #include "llvm/ADT/StringRef.h"
> @@ -81,6 +82,7 @@
>  #include "llvm/Support/Compiler.h"
>  #include "llvm/Support/ErrorHandling.h"
>  #include "llvm/Support/MathExtras.h"
> +#include "llvm/Support/VirtualFileSystem.h"
>  #include "llvm/Support/raw_ostream.h"
>  #include 
>  #include 
> @@ -826,6 +828,18 @@ static bool isAddrSpaceMapManglingEnable
>llvm_unreachable("getAddressSpaceMapMangling() doesn't cover
> anything.");
>  }
>
> +static std::vector
> +getRealPaths(llvm::vfs::FileSystem &VFS, llvm::ArrayRef
> Paths) {
> +  std::vector Result;
> +  llvm::SmallString<128> Buffer;
> +  for (const auto &File : Paths) {
> +if (std::error_code EC = VFS.getRealPath(File, Buffer))
> +  llvm::report_fatal_error("can't open file '" + File + "': " +
> EC.message());
> +Result.push_back(Buffer.str());
> +  }
> +  return Result;
> +}
> +
>  ASTContext::ASTContext(LangOptions &LOpts, SourceManager &SM,
> IdentifierTable &idents, SelectorTable &sels,
> Builtin::Context &builtins)
> @@ -833,7 +847,10 @@ ASTContext::ASTContext(LangOptions &LOpt
>TemplateSpecializationTypes(this_()),
>DependentTemplateSpecializationTypes(this_()),
>SubstTemplateTemplateParmPacks(this_()), SourceMgr(SM),
> LangOpts(LOpts),
> -  SanitizerBL(new
> SanitizerBlacklist(LangOpts.SanitizerBlacklistFiles, SM)),
> +  SanitizerBL(new SanitizerBlacklist(
> +  getRealPaths(SM.getFileManager().getVirtualFileSystem(),
> +   LangOpts.SanitizerBlacklistFiles),
> +  SM)),
>XRayFilter(new
> XRayFunctionFilter(LangOpts.XRayAlwaysInstrumentFiles,
>  LangOpts.XRayNeverInstrumentFiles,
>  LangOpts.XRayAttrListFiles, SM)),
>
> Added: cfe/trunk/test/CodeGen/Inputs/sanitizer-blacklist-vfsoverlay.yaml
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/Inputs/sanitizer-blacklist-vfsoverlay.yaml?rev=374006&view=auto
>
> ==
> --- cfe/trunk/test/CodeGen/Inputs/sanitizer-blacklist-vfsoverlay.yaml
> (added)
> +++ cfe/trunk/test/CodeGen/Inputs/sanitizer-blacklist-vfsoverlay.yaml Mon
> Oct  7 18:13:17 2019
> @@ -0,0 +1,15 @@
> +{
> +  'version': 0,
> +  'roots': [
> +{ 'name': '@DIR@', 'type': 'directory',
> +  'contents': [
> +{ 'name': 'only-virtual-file.blacklist', 'type': 'file',
> +  'external-contents': '@REAL_FILE@'
> +},
> +{ 'name': 'invalid-virtual-file.blacklist', 'type': 'file',
> +  'external-contents': '@NONEXISTENT_FILE@'
> +}
> +  ]
> +}
> +  ]
> +}
>
> Modified: cfe/trunk/test/CodeGen/ubsan-blacklist.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ubsan-blacklist.c?rev=374006&r1=374005&r2=374006&view=diff
>
> ==
> --- cfe/trunk/test/CodeGen/ubsan-blacklist.c (original)
> +++ cfe/trunk/test/CodeGen/ubsan-blacklist.c Mon Oct  7 18:13:17 2019
> @@ -5,6 +5,17 @@
>  // RUN: %clang_cc1 -fsanitize=unsigned-integer-overflow
> -fsanitize-blacklist=%t-func.blacklist -emit-llvm %s -o - | FileCheck %s
> --ch

r374148 - Unify the two CRC implementations

[Hans Wennborg via cfe-commits]

Author: hans
Date: Wed Oct  9 02:06:30 2019
New Revision: 374148

URL: http://llvm.org/viewvc/llvm-project?rev=374148&view=rev
Log:
Unify the two CRC implementations

David added the JamCRC implementation in r246590. More recently, Eugene
added a CRC-32 implementation in r357901, which falls back to zlib's
crc32 function if present.

These checksums are essentially the same, so having multiple
implementations seems unnecessary. This replaces the CRC-32
implementation with the simpler one from JamCRC, and implements the
JamCRC interface in terms of CRC-32 since this means it can use zlib's
implementation when available, saving a few bytes and potentially making
it faster.

JamCRC took an ArrayRef argument, and CRC-32 took a StringRef.
This patch changes it to ArrayRef which I think is the best
choice, and simplifies a few of the callers nicely.

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

Modified:
cfe/trunk/lib/AST/MicrosoftMangle.cpp

Modified: cfe/trunk/lib/AST/MicrosoftMangle.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/MicrosoftMangle.cpp?rev=374148&r1=374147&r2=374148&view=diff
==
--- cfe/trunk/lib/AST/MicrosoftMangle.cpp (original)
+++ cfe/trunk/lib/AST/MicrosoftMangle.cpp Wed Oct  9 02:06:30 2019
@@ -27,11 +27,11 @@
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/TargetInfo.h"
 #include "llvm/ADT/StringExtras.h"
-#include "llvm/Support/JamCRC.h"
-#include "llvm/Support/xxhash.h"
+#include "llvm/Support/CRC.h"
 #include "llvm/Support/MD5.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/StringSaver.h"
+#include "llvm/Support/xxhash.h"
 
 using namespace clang;
 


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

[PATCH] D68570: Unify the two CRC implementations

[Hans via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1e1e3ba2526e: Unify the two CRC implementations (authored by 
hansw).
Herald added projects: clang, LLDB.
Herald added subscribers: lldb-commits, cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68570

Files:
  clang/lib/AST/MicrosoftMangle.cpp
  lld/COFF/PDB.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  llvm/include/llvm/Support/CRC.h
  llvm/include/llvm/Support/JamCRC.h
  llvm/lib/DebugInfo/PDB/Native/Hash.cpp
  llvm/lib/DebugInfo/PDB/Native/PDBFileBuilder.cpp
  llvm/lib/DebugInfo/PDB/Native/TpiHashing.cpp
  llvm/lib/DebugInfo/Symbolize/Symbolize.cpp
  llvm/lib/MC/WinCOFFObjectWriter.cpp
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CRC.cpp
  llvm/lib/Support/JamCRC.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/tools/llvm-objcopy/COFF/COFFObjcopy.cpp
  llvm/tools/llvm-objcopy/CopyConfig.cpp
  llvm/unittests/Support/CRCTest.cpp
  llvm/utils/gn/secondary/llvm/lib/Support/BUILD.gn

Index: llvm/utils/gn/secondary/llvm/lib/Support/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/lib/Support/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/lib/Support/BUILD.gn
@@ -79,7 +79,6 @@
 "IntervalMap.cpp",
 "ItaniumManglingCanonicalizer.cpp",
 "JSON.cpp",
-"JamCRC.cpp",
 "KnownBits.cpp",
 "LEB128.cpp",
 "LineIterator.cpp",
Index: llvm/unittests/Support/CRCTest.cpp
===
--- llvm/unittests/Support/CRCTest.cpp
+++ llvm/unittests/Support/CRCTest.cpp
@@ -11,6 +11,7 @@
 //===--===//
 
 #include "llvm/Support/CRC.h"
+#include "llvm/ADT/StringExtras.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
@@ -18,12 +19,26 @@
 namespace {
 
 TEST(CRCTest, CRC32) {
-  EXPECT_EQ(0x414FA339U,
-llvm::crc32(
-0, StringRef("The quick brown fox jumps over the lazy dog")));
+  EXPECT_EQ(0x414FA339U, llvm::crc32(arrayRefFromStringRef(
+ "The quick brown fox jumps over the lazy dog")));
+
   // CRC-32/ISO-HDLC test vector
   // http://reveng.sourceforge.net/crc-catalogue/17plus.htm#crc.cat.crc-32c
-  EXPECT_EQ(0xCBF43926U, llvm::crc32(0, StringRef("123456789")));
+  EXPECT_EQ(0xCBF43926U, llvm::crc32(arrayRefFromStringRef("123456789")));
+
+  // Check the CRC-32 of each byte value, exercising all of CRCTable.
+  for (int i = 0; i < 256; i++) {
+// Compute CRCTable[i] using Hacker's Delight (2nd ed.) Figure 14-7.
+uint32_t crc = i;
+for (int j = 7; j >= 0; j--) {
+  uint32_t mask = -(crc & 1);
+  crc = (crc >> 1) ^ (0xEDB88320 & mask);
+}
+
+// CRCTable[i] is the CRC-32 of i without the initial and final bit flips.
+uint8_t byte = i;
+EXPECT_EQ(crc, ~llvm::crc32(0xU, byte));
+  }
 }
 
 } // end anonymous namespace
Index: llvm/tools/llvm-objcopy/CopyConfig.cpp
===
--- llvm/tools/llvm-objcopy/CopyConfig.cpp
+++ llvm/tools/llvm-objcopy/CopyConfig.cpp
@@ -14,10 +14,10 @@
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Option/ArgList.h"
+#include "llvm/Support/CRC.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Compression.h"
 #include "llvm/Support/Errc.h"
-#include "llvm/Support/JamCRC.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/StringSaver.h"
 #include 
@@ -461,12 +461,8 @@
 if (!DebugOrErr)
   return createFileError(Config.AddGnuDebugLink, DebugOrErr.getError());
 auto Debug = std::move(*DebugOrErr);
-JamCRC CRC;
-CRC.update(
-ArrayRef(Debug->getBuffer().data(), Debug->getBuffer().size()));
-// The CRC32 value needs to be complemented because the JamCRC doesn't
-// finalize the CRC32 value.
-Config.GnuDebugLinkCRC32 = ~CRC.getCRC();
+Config.GnuDebugLinkCRC32 =
+llvm::crc32(arrayRefFromStringRef(Debug->getBuffer()));
   }
   Config.BuildIdLinkDir = InputArgs.getLastArgValue(OBJCOPY_build_id_link_dir);
   if (InputArgs.hasArg(OBJCOPY_build_id_link_input))
Index: llvm/tools/llvm-objcopy/COFF/COFFObjcopy.cpp
===
--- llvm/tools/llvm-objcopy/COFF/COFFObjcopy.cpp
+++ llvm/tools/llvm-objcopy/COFF/COFFObjcopy.cpp
@@ -16,8 +16,8 @@
 
 #include "llvm/Object/Binary.h"
 #include "llvm/Object/COFF.h"
+#include "llvm/Support/CRC.h"
 #include "llvm/Support/Errc.h"
-#include "llvm/Support/JamCRC.h"
 #include "llvm/Support/Path.h"
 #include 
 
@@ -40,22 +40,13 @@
  Obj.IsPE ? Obj.PeHeader.SectionAlignment : 1);
 }
 
-static uint32_t getCRC32(StringRef Data) {
-  JamCRC CRC;
-  CRC.update(ArrayRef(Data.data(), Data.size()));
-  // The CRC32 value needs to be 

r374151 - Revert r374006: Reland 'Add VFS support for sanitizers' blacklist'

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Wed Oct  9 02:40:22 2019
New Revision: 374151

URL: http://llvm.org/viewvc/llvm-project?rev=374151&view=rev
Log:
Revert r374006: Reland 'Add VFS support for sanitizers' blacklist'

Also revert follow-up changes to the test.
Reason: the patch breaks our internal clang-tidy integration.

It's also unclear why we should use getRealPath instead of plumbing the
VFS to SanitizerBlacklist, see original commit thread of cfe-commits for
a discussion.

Removed:
cfe/trunk/test/CodeGen/Inputs/sanitizer-blacklist-vfsoverlay.yaml
cfe/trunk/test/CodeGen/ubsan-blacklist-vfs.c
Modified:
cfe/trunk/lib/AST/ASTContext.cpp

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=374151&r1=374150&r2=374151&view=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Wed Oct  9 02:40:22 2019
@@ -72,7 +72,6 @@
 #include "llvm/ADT/PointerUnion.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallPtrSet.h"
-#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
@@ -82,7 +81,6 @@
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MathExtras.h"
-#include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
@@ -828,18 +826,6 @@ static bool isAddrSpaceMapManglingEnable
   llvm_unreachable("getAddressSpaceMapMangling() doesn't cover anything.");
 }
 
-static std::vector
-getRealPaths(llvm::vfs::FileSystem &VFS, llvm::ArrayRef Paths) {
-  std::vector Result;
-  llvm::SmallString<128> Buffer;
-  for (const auto &File : Paths) {
-if (std::error_code EC = VFS.getRealPath(File, Buffer))
-  llvm::report_fatal_error("can't open file '" + File + "': " + 
EC.message());
-Result.push_back(Buffer.str());
-  }
-  return Result;
-}
-
 ASTContext::ASTContext(LangOptions &LOpts, SourceManager &SM,
IdentifierTable &idents, SelectorTable &sels,
Builtin::Context &builtins)
@@ -847,10 +833,7 @@ ASTContext::ASTContext(LangOptions &LOpt
   TemplateSpecializationTypes(this_()),
   DependentTemplateSpecializationTypes(this_()),
   SubstTemplateTemplateParmPacks(this_()), SourceMgr(SM), LangOpts(LOpts),
-  SanitizerBL(new SanitizerBlacklist(
-  getRealPaths(SM.getFileManager().getVirtualFileSystem(),
-   LangOpts.SanitizerBlacklistFiles),
-  SM)),
+  SanitizerBL(new SanitizerBlacklist(LangOpts.SanitizerBlacklistFiles, 
SM)),
   XRayFilter(new XRayFunctionFilter(LangOpts.XRayAlwaysInstrumentFiles,
 LangOpts.XRayNeverInstrumentFiles,
 LangOpts.XRayAttrListFiles, SM)),

Removed: cfe/trunk/test/CodeGen/Inputs/sanitizer-blacklist-vfsoverlay.yaml
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/Inputs/sanitizer-blacklist-vfsoverlay.yaml?rev=374150&view=auto
==
--- cfe/trunk/test/CodeGen/Inputs/sanitizer-blacklist-vfsoverlay.yaml (original)
+++ cfe/trunk/test/CodeGen/Inputs/sanitizer-blacklist-vfsoverlay.yaml (removed)
@@ -1,15 +0,0 @@
-{
-  'version': 0,
-  'roots': [
-{ 'name': '@DIR@', 'type': 'directory',
-  'contents': [
-{ 'name': 'only-virtual-file.blacklist', 'type': 'file',
-  'external-contents': '@REAL_FILE@'
-},
-{ 'name': 'invalid-virtual-file.blacklist', 'type': 'file',
-  'external-contents': '@NONEXISTENT_FILE@'
-}
-  ]
-}
-  ]
-}

Removed: cfe/trunk/test/CodeGen/ubsan-blacklist-vfs.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ubsan-blacklist-vfs.c?rev=374150&view=auto
==
--- cfe/trunk/test/CodeGen/ubsan-blacklist-vfs.c (original)
+++ cfe/trunk/test/CodeGen/ubsan-blacklist-vfs.c (removed)
@@ -1,36 +0,0 @@
-// Verify ubsan doesn't emit checks for blacklisted functions and files
-// RUN: echo "fun:hash" > %t-func.blacklist
-// RUN: echo "src:%s" | sed -e 's/\\//g' > %t-file.blacklist
-
-// RUN: rm -f %t-vfsoverlay.yaml
-// RUN: rm -f %t-nonexistent.blacklist
-// RUN: sed -e "s|@DIR@|%/T|g" %S/Inputs/sanitizer-blacklist-vfsoverlay.yaml | 
sed -e "s|@REAL_FILE@|%/t-func.blacklist|g" | sed -e 
"s|@NONEXISTENT_FILE@|%/t-nonexistent.blacklist|g" > %t-vfsoverlay.yaml
-// RUN: %clang_cc1 -fsanitize=unsigned-integer-overflow -ivfsoverlay 
%t-vfsoverlay.yaml -fsanitize-blacklist=%/T/only-virtual-file.blacklist 
-emit-llvm %s -o - | FileCheck %s --check-prefix=FUNC
-
-// RUN: not %clang_cc1 -fsanitize=unsigned-integer-overflow -ivfsoverlay 
%t-vfsoverlay.yaml -fsanitize-blacklist=%/T/invalid-virtual-file.blacklist 
-emit-l

Re: r374006 - Reland 'Add VFS support for sanitizers' blacklist'

[Ilya Biryukov via cfe-commits]

Reverted in r374151.

On Wed, Oct 9, 2019 at 11:03 AM Ilya Biryukov  wrote:

> Hi Jan,
>
> This patch seems to assume VFS will only re-map some paths, but still
> point into the physical filesystem.
> This may not be the case, e.g. in unit tests.
>
> This also manages to break some of our internal clang-tidy integrations
> for obscure reasons.
>
> Can we instead just pass VFS instance to the SanitizerBlacklist and use
> relative paths?
>
> We might also need to revert the patch to unbreak our release, sorry about
> the inconvenience.
>
> On Tue, Oct 8, 2019 at 3:11 AM Jan Korous via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: jkorous
>> Date: Mon Oct  7 18:13:17 2019
>> New Revision: 374006
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=374006&view=rev
>> Log:
>> Reland 'Add VFS support for sanitizers' blacklist'
>>
>> The original patch broke the test for Windows.
>> Trying to fix as per Reid's suggestions outlined here:
>> https://reviews.llvm.org/rC371663
>>
>> Differential Revision: https://reviews.llvm.org/D67742
>>
>> Added:
>> cfe/trunk/test/CodeGen/Inputs/sanitizer-blacklist-vfsoverlay.yaml
>> Modified:
>> cfe/trunk/lib/AST/ASTContext.cpp
>> cfe/trunk/test/CodeGen/ubsan-blacklist.c
>>
>> Modified: cfe/trunk/lib/AST/ASTContext.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=374006&r1=374005&r2=374006&view=diff
>>
>> ==
>> --- cfe/trunk/lib/AST/ASTContext.cpp (original)
>> +++ cfe/trunk/lib/AST/ASTContext.cpp Mon Oct  7 18:13:17 2019
>> @@ -72,6 +72,7 @@
>>  #include "llvm/ADT/PointerUnion.h"
>>  #include "llvm/ADT/STLExtras.h"
>>  #include "llvm/ADT/SmallPtrSet.h"
>> +#include "llvm/ADT/SmallString.h"
>>  #include "llvm/ADT/SmallVector.h"
>>  #include "llvm/ADT/StringExtras.h"
>>  #include "llvm/ADT/StringRef.h"
>> @@ -81,6 +82,7 @@
>>  #include "llvm/Support/Compiler.h"
>>  #include "llvm/Support/ErrorHandling.h"
>>  #include "llvm/Support/MathExtras.h"
>> +#include "llvm/Support/VirtualFileSystem.h"
>>  #include "llvm/Support/raw_ostream.h"
>>  #include 
>>  #include 
>> @@ -826,6 +828,18 @@ static bool isAddrSpaceMapManglingEnable
>>llvm_unreachable("getAddressSpaceMapMangling() doesn't cover
>> anything.");
>>  }
>>
>> +static std::vector
>> +getRealPaths(llvm::vfs::FileSystem &VFS, llvm::ArrayRef
>> Paths) {
>> +  std::vector Result;
>> +  llvm::SmallString<128> Buffer;
>> +  for (const auto &File : Paths) {
>> +if (std::error_code EC = VFS.getRealPath(File, Buffer))
>> +  llvm::report_fatal_error("can't open file '" + File + "': " +
>> EC.message());
>> +Result.push_back(Buffer.str());
>> +  }
>> +  return Result;
>> +}
>> +
>>  ASTContext::ASTContext(LangOptions &LOpts, SourceManager &SM,
>> IdentifierTable &idents, SelectorTable &sels,
>> Builtin::Context &builtins)
>> @@ -833,7 +847,10 @@ ASTContext::ASTContext(LangOptions &LOpt
>>TemplateSpecializationTypes(this_()),
>>DependentTemplateSpecializationTypes(this_()),
>>SubstTemplateTemplateParmPacks(this_()), SourceMgr(SM),
>> LangOpts(LOpts),
>> -  SanitizerBL(new
>> SanitizerBlacklist(LangOpts.SanitizerBlacklistFiles, SM)),
>> +  SanitizerBL(new SanitizerBlacklist(
>> +  getRealPaths(SM.getFileManager().getVirtualFileSystem(),
>> +   LangOpts.SanitizerBlacklistFiles),
>> +  SM)),
>>XRayFilter(new
>> XRayFunctionFilter(LangOpts.XRayAlwaysInstrumentFiles,
>>
>>  LangOpts.XRayNeverInstrumentFiles,
>>  LangOpts.XRayAttrListFiles, SM)),
>>
>> Added: cfe/trunk/test/CodeGen/Inputs/sanitizer-blacklist-vfsoverlay.yaml
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/Inputs/sanitizer-blacklist-vfsoverlay.yaml?rev=374006&view=auto
>>
>> ==
>> --- cfe/trunk/test/CodeGen/Inputs/sanitizer-blacklist-vfsoverlay.yaml
>> (added)
>> +++ cfe/trunk/test/CodeGen/Inputs/sanitizer-blacklist-vfsoverlay.yaml Mon
>> Oct  7 18:13:17 2019
>> @@ -0,0 +1,15 @@
>> +{
>> +  'version': 0,
>> +  'roots': [
>> +{ 'name': '@DIR@', 'type': 'directory',
>> +  'contents': [
>> +{ 'name': 'only-virtual-file.blacklist', 'type': 'file',
>> +  'external-contents': '@REAL_FILE@'
>> +},
>> +{ 'name': 'invalid-virtual-file.blacklist', 'type': 'file',
>> +  'external-contents': '@NONEXISTENT_FILE@'
>> +}
>> +  ]
>> +}
>> +  ]
>> +}
>>
>> Modified: cfe/trunk/test/CodeGen/ubsan-blacklist.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ubsan-blacklist.c?rev=374006&r1=374005&r2=374006&view=diff
>>
>> ==
>> --- cfe/trunk/test/CodeGen/ubsan-blacklist.c (original)
>> +++ cfe/trunk/test/CodeGen/ubsan-blacklist.c 

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D64799#1688293 , @dgoldman wrote:

> In D64799#1605514 , @ilya-biryukov 
> wrote:
>
> > Tried the suggested approach and ran into the issue described above.
> >  Either we make the diagnostics less useful ('did you mean foo::bar?' turns 
> > into 'unresolved identifier bar'; without mentioning the correction)  or we 
> > even stop providing some corrections in addition to that.
> >
> > On the other hand, I agree that over time we will start emitting too many 
> > diagnostics at the end of TU if keep the patch as is. That is not a good 
> > way.
> >  There should be a better option for emitting the uncorrected diagnostics 
> > closer to where they are produced, but I don't have a good idea on what it 
> > should be off the top of my head. Ideas warmly welcome.
>
>
> I might be wrong here, but I thought that diagnostics were delayed until typo 
> correction has been completed on an expression. For example, you'll only get 
> something like `unresolved identifier bar` instead of `did you mean 
> foo::bar?` only when you call the `DiagHandler` with either a proper 
> `TypoCorrection` or an empty one. If so, I'm not sure how you'd get into this 
> case if you're calling `CorrectDelayedTyposInExpr` and tracking which typos 
> have been resolved already.


I couldn't find a place in the code that would make sure the typo expressions 
will not be corrected later.
Therefore, this approach led to worse diagnostics (`unresolved identifier bar` 
vs `did you mean baz?`) in some situations, i.e. when a correction would take 
place somewhere higher up the callstack, but we decided to emit the error 
before that.

In particular, I tried emitting uncorrected typo diagnostics when popping 
expression evaluation contexts, per Richard's suggestion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64799



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

r374152 - [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Wed Oct  9 03:00:05 2019
New Revision: 374152

URL: http://llvm.org/viewvc/llvm-project?rev=374152&view=rev
Log:
[Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

Summary:
Instead of asserting all typos are corrected in the sema destructor.

The sema destructor is not run in the common case of running the compiler
with the -disable-free cc1 flag (which is the default in the driver).

Having this assertion led to crashes in libclang and clangd, which are not
reproducible when running the compiler.

Asserting at the end of the TU could be an option, but finding all
missing typo correction cases is hard and having worse diagnostics instead
of a failing assertion is a better trade-off.

For more discussion on this, see:
https://lists.llvm.org/pipermail/cfe-dev/2019-July/062872.html

Reviewers: sammccall, rsmith

Reviewed By: rsmith

Subscribers: usaxena95, dgoldman, jkorous, vsapsai, rnk, kadircet, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/Sema/Sema.cpp
cfe/trunk/test/SemaObjC/typo-correction-subscript.m

Modified: cfe/trunk/lib/Sema/Sema.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=374152&r1=374151&r2=374152&view=diff
==
--- cfe/trunk/lib/Sema/Sema.cpp (original)
+++ cfe/trunk/lib/Sema/Sema.cpp Wed Oct  9 03:00:05 2019
@@ -38,6 +38,7 @@
 #include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/TemplateDeduction.h"
 #include "clang/Sema/TemplateInstCallback.h"
+#include "clang/Sema/TypoCorrection.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/Support/TimeProfiler.h"
@@ -383,8 +384,6 @@ Sema::~Sema() {
   // Detach from the PP callback handler which outlives Sema since it's owned
   // by the preprocessor.
   SemaPPCallbackHandler->reset();
-
-  assert(DelayedTypos.empty() && "Uncorrected typos!");
 }
 
 void Sema::warnStackExhausted(SourceLocation Loc) {
@@ -934,6 +933,15 @@ void Sema::ActOnEndOfTranslationUnitFrag
   assert(LateParsedInstantiations.empty() &&
  "end of TU template instantiation should not create more "
  "late-parsed templates");
+
+  // Report diagnostics for uncorrected delayed typos. Ideally all of them
+  // should have been corrected by that time, but it is very hard to cover all
+  // cases in practice.
+  for (const auto &Typo : DelayedTypos) {
+// We pass an empty TypoCorrection to indicate no correction was performed.
+Typo.second.DiagHandler(TypoCorrection());
+  }
+  DelayedTypos.clear();
 }
 
 /// ActOnEndOfTranslationUnit - This is called at the very end of the

Modified: cfe/trunk/test/SemaObjC/typo-correction-subscript.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/typo-correction-subscript.m?rev=374152&r1=374151&r2=374152&view=diff
==
--- cfe/trunk/test/SemaObjC/typo-correction-subscript.m (original)
+++ cfe/trunk/test/SemaObjC/typo-correction-subscript.m Wed Oct  9 03:00:05 2019
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple i386-apple-macosx10.10 -fobjc-arc -fsyntax-only 
-Wno-objc-root-class %s -verify -disable-free
+// RUN: %clang_cc1 -triple i386-apple-macosx10.10 -fobjc-arc -fsyntax-only 
-Wno-objc-root-class %s -verify
 
 @class Dictionary;
 
@@ -9,6 +9,7 @@
 - (void)rdar47403222:(Dictionary *)opts {
   [self undeclaredMethod:undeclaredArg];
   // expected-error@-1{{no visible @interface for 'Test' declares the selector 
'undeclaredMethod:'}}
+  // expected-error@-2{{use of undeclared identifier 'undeclaredArg}}
   opts[(__bridge id)undeclaredKey] = 0;
   // expected-error@-1{{use of undeclared identifier 'undeclaredKey'}}
 }


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

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang/lib/Sema/Sema.cpp:916
+  // cases in practice.
+  for (const auto &Typo : DelayedTypos) {
+// We pass an empty TypoCorrection to indicate no correction was performed.

sammccall wrote:
> If you like, `DEBUG_WITH_TYPE("DelayedTypos", ...)` would still let people 
> who want to fix these see them. In practice, I suspect nobody actually wants 
> to fix these, though :-(
I've left it out for now, not sure what's the best way to report those: crash? 
show a message in stderr? (probably a short message, indicating the number of 
uncorrected delayed typos in stderr is enough)
Also second the concerns that it's not very useful since no-one is actually 
looking at these.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64799



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

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

[Ilya Biryukov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGaeae71cd96c3: [Sema] Emit diagnostics for uncorrected 
delayed typos at the end of TU (authored by ilya-biryukov).

Changed prior to commit:
  https://reviews.llvm.org/D64799?vs=210338&id=224005#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64799

Files:
  clang/lib/Sema/Sema.cpp
  clang/test/SemaObjC/typo-correction-subscript.m


Index: clang/test/SemaObjC/typo-correction-subscript.m
===
--- clang/test/SemaObjC/typo-correction-subscript.m
+++ clang/test/SemaObjC/typo-correction-subscript.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple i386-apple-macosx10.10 -fobjc-arc -fsyntax-only 
-Wno-objc-root-class %s -verify -disable-free
+// RUN: %clang_cc1 -triple i386-apple-macosx10.10 -fobjc-arc -fsyntax-only 
-Wno-objc-root-class %s -verify
 
 @class Dictionary;
 
@@ -9,6 +9,7 @@
 - (void)rdar47403222:(Dictionary *)opts {
   [self undeclaredMethod:undeclaredArg];
   // expected-error@-1{{no visible @interface for 'Test' declares the selector 
'undeclaredMethod:'}}
+  // expected-error@-2{{use of undeclared identifier 'undeclaredArg}}
   opts[(__bridge id)undeclaredKey] = 0;
   // expected-error@-1{{use of undeclared identifier 'undeclaredKey'}}
 }
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -38,6 +38,7 @@
 #include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/TemplateDeduction.h"
 #include "clang/Sema/TemplateInstCallback.h"
+#include "clang/Sema/TypoCorrection.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/Support/TimeProfiler.h"
@@ -383,8 +384,6 @@
   // Detach from the PP callback handler which outlives Sema since it's owned
   // by the preprocessor.
   SemaPPCallbackHandler->reset();
-
-  assert(DelayedTypos.empty() && "Uncorrected typos!");
 }
 
 void Sema::warnStackExhausted(SourceLocation Loc) {
@@ -934,6 +933,15 @@
   assert(LateParsedInstantiations.empty() &&
  "end of TU template instantiation should not create more "
  "late-parsed templates");
+
+  // Report diagnostics for uncorrected delayed typos. Ideally all of them
+  // should have been corrected by that time, but it is very hard to cover all
+  // cases in practice.
+  for (const auto &Typo : DelayedTypos) {
+// We pass an empty TypoCorrection to indicate no correction was performed.
+Typo.second.DiagHandler(TypoCorrection());
+  }
+  DelayedTypos.clear();
 }
 
 /// ActOnEndOfTranslationUnit - This is called at the very end of the


Index: clang/test/SemaObjC/typo-correction-subscript.m
===
--- clang/test/SemaObjC/typo-correction-subscript.m
+++ clang/test/SemaObjC/typo-correction-subscript.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple i386-apple-macosx10.10 -fobjc-arc -fsyntax-only -Wno-objc-root-class %s -verify -disable-free
+// RUN: %clang_cc1 -triple i386-apple-macosx10.10 -fobjc-arc -fsyntax-only -Wno-objc-root-class %s -verify
 
 @class Dictionary;
 
@@ -9,6 +9,7 @@
 - (void)rdar47403222:(Dictionary *)opts {
   [self undeclaredMethod:undeclaredArg];
   // expected-error@-1{{no visible @interface for 'Test' declares the selector 'undeclaredMethod:'}}
+  // expected-error@-2{{use of undeclared identifier 'undeclaredArg}}
   opts[(__bridge id)undeclaredKey] = 0;
   // expected-error@-1{{use of undeclared identifier 'undeclaredKey'}}
 }
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -38,6 +38,7 @@
 #include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/TemplateDeduction.h"
 #include "clang/Sema/TemplateInstCallback.h"
+#include "clang/Sema/TypoCorrection.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/Support/TimeProfiler.h"
@@ -383,8 +384,6 @@
   // Detach from the PP callback handler which outlives Sema since it's owned
   // by the preprocessor.
   SemaPPCallbackHandler->reset();
-
-  assert(DelayedTypos.empty() && "Uncorrected typos!");
 }
 
 void Sema::warnStackExhausted(SourceLocation Loc) {
@@ -934,6 +933,15 @@
   assert(LateParsedInstantiations.empty() &&
  "end of TU template instantiation should not create more "
  "late-parsed templates");
+
+  // Report diagnostics for uncorrected delayed typos. Ideally all of them
+  // should have been corrected by that time, but it is very hard to cover all
+  // cases in practice.
+  for (const auto &Typo : DelayedTypos) {
+// We pass an empty TypoCorrection to indicate no correction was performed.
+Typo.second.DiagHandler(TypoCorrection());
+  }
+  DelayedTypos.clear();
 }
 
 /// ActOnEndOfTranslationUnit - This is called at the very end o

[PATCH] D68562: [clangd] Add RemoveUsingNamespace tweak.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp:87
+return false;
+  if (const Decl *ParentDecl = Node->Parent->ASTNode.get())
+return llvm::isa(ParentDecl);

Can we use `ASTNode.get()` to directly to check for this?

Not sure how `DynTypedNode` works, though, maybe that's impossible.



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp:100
+bool isInsideNamespace(const DeclContext *D, const NamespaceDecl *Target) {
+  while (D && (D->isNamespace() || D->isTransparentContext())) {
+if (D->Equals(Target))

Hm, I'd expect us to simply go up until the first non-trasparent namespace and 
stop at it.
Consider the following example:
```
namespace a { namespace b {
  struct Foo {};
}}

using namespace a; // <-- remove this
using namespace b;

Foo x; // <-- Foo does not need to be qualified, as it's inside b.
```

OTOH, in case of inline namespace we'd choose to qualify:
```
namespace a { inline namespace b {
  struct Foo {};
}}

using namespace a; // <-- remove this

Foo x; // <-- need to change to a::Foo
```

Obviously, if we had `using namespace a::b` we'd not want to qualify, but 
that's complicated. And it's very rare to have `using namespace` for an inline 
namespace.



Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:782
+  int main() {
+aa::map m;
+  }

This is incorrect, right? We should not be qualifying here.
See the relevant comment on `isInsideNamespace`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68562



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

[PATCH] D67004: [DebugInfo] Enable call site parameter debug info for ARM and AArch64

[Nikola Prica via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf71bac6f4351: [DebugInfo] Enable call site debug info for 
ARM and AArch64 (authored by NikolaPrica).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67004?vs=218306&id=224006#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67004

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/debug-info-param-modification.c


Index: clang/test/CodeGen/debug-info-param-modification.c
===
--- clang/test/CodeGen/debug-info-param-modification.c
+++ clang/test/CodeGen/debug-info-param-modification.c
@@ -1,4 +1,8 @@
 // RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -Xclang 
-disable-llvm-passes -S -target x86_64-none-linux-gnu -emit-llvm %s -o - | 
FileCheck %s -check-prefix=CHECK-ENTRY-VAL-OPT
+// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -Xclang 
-disable-llvm-passes -S -target arm-none-linux-gnu -emit-llvm %s -o - | 
FileCheck %s -check-prefix=CHECK-ENTRY-VAL-OPT
+// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -Xclang 
-disable-llvm-passes -S -target aarch64-none-linux-gnu -emit-llvm %s -o - | 
FileCheck %s -check-prefix=CHECK-ENTRY-VAL-OPT
+// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -Xclang 
-disable-llvm-passes -S -target armeb-none-linux-gnu -emit-llvm %s -o - | 
FileCheck %s -check-prefix=CHECK-ENTRY-VAL-OPT
+
 // CHECK-ENTRY-VAL-OPT: !DILocalVariable(name: "a", arg: 1, scope: {{.*}}, 
file: {{.*}}, line: {{.*}}, type: {{.*}})
 // CHECK-ENTRY-VAL-OPT: !DILocalVariable(name: "b", arg: 2, scope: {{.*}}, 
file: {{.*}}, line: {{.*}}, type: {{.*}}, flags: DIFlagArgumentNotModified)
 //
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -777,10 +777,14 @@
   Opts.DisableLLVMPasses = Args.hasArg(OPT_disable_llvm_passes);
   Opts.DisableLifetimeMarkers = Args.hasArg(OPT_disable_lifetimemarkers);
 
+  const llvm::Triple::ArchType DebugEntryValueArchs[] = {
+  llvm::Triple::x86, llvm::Triple::x86_64, llvm::Triple::aarch64,
+  llvm::Triple::arm, llvm::Triple::armeb};
+
   llvm::Triple T(TargetOpts.Triple);
-  llvm::Triple::ArchType Arch = T.getArch();
   if (Opts.OptimizationLevel > 0 &&
-  (Arch == llvm::Triple::x86 || Arch == llvm::Triple::x86_64))
+  Opts.getDebugInfo() >= codegenoptions::LimitedDebugInfo &&
+  llvm::is_contained(DebugEntryValueArchs, T.getArch()))
 Opts.EnableDebugEntryValues = Args.hasArg(OPT_femit_debug_entry_values);
 
   Opts.DisableO0ImplyOptNone = Args.hasArg(OPT_disable_O0_optnone);
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3706,8 +3706,7 @@
   const FunctionDecl *CalleeDecl) {
   auto &CGOpts = CGM.getCodeGenOpts();
   if (!CGOpts.EnableDebugEntryValues || !CGM.getLangOpts().Optimize ||
-  !CallOrInvoke ||
-  CGM.getCodeGenOpts().getDebugInfo() < codegenoptions::LimitedDebugInfo)
+  !CallOrInvoke)
 return;
 
   auto *Func = CallOrInvoke->getCalledFunction();


Index: clang/test/CodeGen/debug-info-param-modification.c
===
--- clang/test/CodeGen/debug-info-param-modification.c
+++ clang/test/CodeGen/debug-info-param-modification.c
@@ -1,4 +1,8 @@
 // RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -Xclang -disable-llvm-passes -S -target x86_64-none-linux-gnu -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK-ENTRY-VAL-OPT
+// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -Xclang -disable-llvm-passes -S -target arm-none-linux-gnu -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK-ENTRY-VAL-OPT
+// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -Xclang -disable-llvm-passes -S -target aarch64-none-linux-gnu -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK-ENTRY-VAL-OPT
+// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -Xclang -disable-llvm-passes -S -target armeb-none-linux-gnu -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK-ENTRY-VAL-OPT
+
 // CHECK-ENTRY-VAL-OPT: !DILocalVariable(name: "a", arg: 1, scope: {{.*}}, file: {{.*}}, line: {{.*}}, type: {{.*}})
 // CHECK-ENTRY-VAL-OPT: !DILocalVariable(name: "b", arg: 2, scope: {{.*}}, file: {{.*}}, line: {{.*}}, type: {{.*}}, flags: DIFlagArgumentNotModified)
 //
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocatio

[PATCH] D67536: [WIP] [clangd] Add support for an inactive regions notification

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Just noticed the next version of LSP added diagnostic tags for things like 
"unused field" or "dead code":
https://microsoft.github.io/language-server-protocol/specifications/specification-3-15,
 search for `DiagnosticTag`.

So I guess we won't need ever add range-based highlightings for dead code (and 
similar), we'll use diagnostics instead.

Concerns about having two kinds of highlightings to handle on the clients are 
still there, though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67536



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

r374154 - [mips] Set default float ABI to "soft" on FreeBSD

[Simon Atanasyan via cfe-commits]

Author: atanasyan
Date: Wed Oct  9 03:38:03 2019
New Revision: 374154

URL: http://llvm.org/viewvc/llvm-project?rev=374154&view=rev
Log:
[mips] Set default float ABI to "soft" on FreeBSD

Initial patch by Kyle Evans.

Fix PR43596

Modified:
cfe/trunk/lib/Driver/ToolChains/Arch/Mips.cpp
cfe/trunk/lib/Driver/ToolChains/Arch/Mips.h
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
cfe/trunk/test/Driver/mips-float.c

Modified: cfe/trunk/lib/Driver/ToolChains/Arch/Mips.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Arch/Mips.cpp?rev=374154&r1=374153&r2=374154&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Arch/Mips.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Arch/Mips.cpp Wed Oct  9 03:38:03 2019
@@ -149,7 +149,8 @@ StringRef mips::getGnuCompatibleMipsABIN
 
 // Select the MIPS float ABI as determined by -msoft-float, -mhard-float,
 // and -mfloat-abi=.
-mips::FloatABI mips::getMipsFloatABI(const Driver &D, const ArgList &Args) {
+mips::FloatABI mips::getMipsFloatABI(const Driver &D, const ArgList &Args,
+ const llvm::Triple &Triple) {
   mips::FloatABI ABI = mips::FloatABI::Invalid;
   if (Arg *A =
   Args.getLastArg(options::OPT_msoft_float, options::OPT_mhard_float,
@@ -172,10 +173,15 @@ mips::FloatABI mips::getMipsFloatABI(con
 
   // If unspecified, choose the default based on the platform.
   if (ABI == mips::FloatABI::Invalid) {
-// Assume "hard", because it's a default value used by gcc.
-// When we start to recognize specific target MIPS processors,
-// we will be able to select the default more correctly.
-ABI = mips::FloatABI::Hard;
+if (Triple.isOSFreeBSD()) {
+  // For FreeBSD, assume "soft" on all flavors of MIPS.
+  ABI = mips::FloatABI::Soft;
+} else {
+  // Assume "hard", because it's a default value used by gcc.
+  // When we start to recognize specific target MIPS processors,
+  // we will be able to select the default more correctly.
+  ABI = mips::FloatABI::Hard;
+}
   }
 
   assert(ABI != mips::FloatABI::Invalid && "must select an ABI");
@@ -274,7 +280,7 @@ void mips::getMIPSTargetFeatures(const D
   Features.push_back("-xgot");
   }
 
-  mips::FloatABI FloatABI = mips::getMipsFloatABI(D, Args);
+  mips::FloatABI FloatABI = mips::getMipsFloatABI(D, Args, Triple);
   if (FloatABI == mips::FloatABI::Soft) {
 // FIXME: Note, this is a hack. We need to pass the selected float
 // mode to the MipsTargetInfoBase to define appropriate macros there.

Modified: cfe/trunk/lib/Driver/ToolChains/Arch/Mips.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Arch/Mips.h?rev=374154&r1=374153&r2=374154&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Arch/Mips.h (original)
+++ cfe/trunk/lib/Driver/ToolChains/Arch/Mips.h Wed Oct  9 03:38:03 2019
@@ -38,7 +38,8 @@ void getMIPSTargetFeatures(const Driver
const llvm::opt::ArgList &Args,
std::vector &Features);
 StringRef getGnuCompatibleMipsABIName(StringRef ABI);
-mips::FloatABI getMipsFloatABI(const Driver &D, const llvm::opt::ArgList 
&Args);
+mips::FloatABI getMipsFloatABI(const Driver &D, const llvm::opt::ArgList &Args,
+   const llvm::Triple &Triple);
 std::string getMipsABILibSuffix(const llvm::opt::ArgList &Args,
 const llvm::Triple &Triple);
 bool hasMipsAbiArg(const llvm::opt::ArgList &Args, const char *Value);

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=374154&r1=374153&r2=374154&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Wed Oct  9 03:38:03 2019
@@ -1672,7 +1672,7 @@ void Clang::AddMIPSTargetArgs(const ArgL
   CmdArgs.push_back("-target-abi");
   CmdArgs.push_back(ABIName.data());
 
-  mips::FloatABI ABI = mips::getMipsFloatABI(D, Args);
+  mips::FloatABI ABI = mips::getMipsFloatABI(D, Args, Triple);
   if (ABI == mips::FloatABI::Soft) {
 // Floating point operations and argument passing are soft.
 CmdArgs.push_back("-msoft-float");

Modified: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Gnu.cpp?rev=374154&r1=374153&r2=374154&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp Wed Oct  9 03:38:03 2019
@@ -823,7 +823,8 @@ void tools::gnutools::Assembler::Constru
   A->render(Args, CmdArgs);
 } else if (mips:

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:4657
+  Builder.GetInsertBlock()->getParent(), PtrTy->getPointerAddressSpace());
+  // Check for overflows unless the GEP got constant-folded,
+  // and only in the default address space

rsmith wrote:
> If we want to split out the "constant folded" case to avoid issuing too many 
> sanitizer traps on bogus but common patterns, we should have another 
> sanitizer group to re-enable those diagnostics for the constant-folded cases. 
> (I'm fine with not doing that in this patch, though.)
I'm not sure about this point, i think i'm gonna leave this as-is for now..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67122



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

[PATCH] D68694: Do not show "use of a signed integer operand with a binary bitwise operator" for positive integer operands

[Vladimir Plyashkun via Phabricator via cfe-commits]

vladimir.plyashkun created this revision.
vladimir.plyashkun added a reviewer: JonasToth.
vladimir.plyashkun added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, mgehre.
Herald added a project: clang.

Hi! Clang-Tidy has been integrated in CLion some time ago, but a lot of users 
started to complain on `hicpp-signed-bitwise` inspection as it produces a lot 
of noise, especially for positive integer literals.

There are already some issues (with discussions) in LLVM tracker with same 
problem:
https://bugs.llvm.org/show_bug.cgi?id=36961
https://bugs.llvm.org/show_bug.cgi?id=43606

In my opinion this check should be disabled in case of integer literals, since 
there are a lot of existing code (even in system libraries) where user can do 
nothing, e.g.:

  #include 
  
  int main() {
  open("file", O_RDONLY | O_NOCTTY); // <-- warning here
  }


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D68694

Files:
  clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp
  clang-tools-extra/test/clang-tidy/hicpp-signed-bitwise.cpp


Index: clang-tools-extra/test/clang-tidy/hicpp-signed-bitwise.cpp
===
--- clang-tools-extra/test/clang-tidy/hicpp-signed-bitwise.cpp
+++ clang-tools-extra/test/clang-tidy/hicpp-signed-bitwise.cpp
@@ -96,10 +96,8 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use of a signed integer operand 
with a binary bitwise operator
   UByte1<<= UByte2; // Ok
 
-  int SignedInt1 = 1 << 12;
-  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use of a signed integer operand 
with a binary bitwise operator
-  int SignedInt2 = 1u << 12;
-  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use of a signed integer operand 
with a binary bitwise operator
+  int SignedInt1 = 1 << 12; // Ok
+  int SignedInt2 = 1u << 12; // Ok
 }
 
 void f1(unsigned char c) {}
@@ -157,8 +155,7 @@
   r = -1 >> -i;
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use of a signed integer operand 
with a binary bitwise operator
 
-  r = ~0;
-  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use of a signed integer operand 
with a unary bitwise operator
+  r = ~0; // Ok
   r = ~0u; // Ok
   k = ~k;  // Ok
 
@@ -231,10 +228,8 @@
 enum EnumConstruction {
   one = 1,
   two = 2,
-  test1 = 1 << 12,
-  // CHECK-MESSAGES: [[@LINE-1]]:11: warning: use of a signed integer operand 
with a binary bitwise operator
+  test1 = 1 << 12, // Ok
   test2 = one << two,
   // CHECK-MESSAGES: [[@LINE-1]]:11: warning: use of a signed integer operand 
with a binary bitwise operator
-  test3 = 1u << 12,
-  // CHECK-MESSAGES: [[@LINE-1]]:11: warning: use of a signed integer operand 
with a binary bitwise operator
+  test3 = 1u << 12, // Ok
 };
Index: clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp
===
--- clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp
+++ clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp
@@ -19,7 +19,9 @@
 
 void SignedBitwiseCheck::registerMatchers(MatchFinder *Finder) {
   const auto SignedIntegerOperand =
-  
expr(ignoringImpCasts(hasType(isSignedInteger(.bind("signed-operand");
+  expr(ignoringImpCasts(hasType(isSignedInteger())),
+   unless(integerLiteral()))
+  .bind("signed-operand");
 
   // The standard [bitmask.types] allows some integral types to be implemented
   // as signed types. Exclude these types from diagnosing for bitwise or(|) and


Index: clang-tools-extra/test/clang-tidy/hicpp-signed-bitwise.cpp
===
--- clang-tools-extra/test/clang-tidy/hicpp-signed-bitwise.cpp
+++ clang-tools-extra/test/clang-tidy/hicpp-signed-bitwise.cpp
@@ -96,10 +96,8 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use of a signed integer operand with a binary bitwise operator
   UByte1<<= UByte2; // Ok
 
-  int SignedInt1 = 1 << 12;
-  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use of a signed integer operand with a binary bitwise operator
-  int SignedInt2 = 1u << 12;
-  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use of a signed integer operand with a binary bitwise operator
+  int SignedInt1 = 1 << 12; // Ok
+  int SignedInt2 = 1u << 12; // Ok
 }
 
 void f1(unsigned char c) {}
@@ -157,8 +155,7 @@
   r = -1 >> -i;
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use of a signed integer operand with a binary bitwise operator
 
-  r = ~0;
-  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use of a signed integer operand with a unary bitwise operator
+  r = ~0; // Ok
   r = ~0u; // Ok
   k = ~k;  // Ok
 
@@ -231,10 +228,8 @@
 enum EnumConstruction {
   one = 1,
   two = 2,
-  test1 = 1 << 12,
-  // CHECK-MESSAGES: [[@LINE-1]]:11: warning: use of a signed integer operand with a binary bitwise operator
+  test1 = 1 << 12, // Ok
   test2 = one << two,
   // CHECK-MESSAGES: [[@LINE-1]]:11: warning: use of a signed integer operand with a binary bitwise operator
-  test3 = 1u

[PATCH] D68634: [ASTImporter] Imported FunctionDecls inherit attributes from existing functions

[Gabor Marton via Phabricator via cfe-commits]

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

In D68634#1700591 , @a_sidorin wrote:

> Hello Balasz,
>  In my opinion, importing and setting the attributes should be handled by the 
> stuff used in InitializeImportedDecl(). Can we extend it or reuse the code? 
> It will allow us not to miss the required actions while importing a new 
> function too.


I was thinking about that too, but it turned out to be a way more intrusive 
change.

We have to work with the most recent existing decl (`PrevDecl`) of the 
FunctionDecl whose attribute we currently import.
We can get the `PrevDecl` only with the help of the lookup. We can't get it 
from the `ToD` param of `InitializeImportedDecl` because by the time when we 
call `InitializeImportedDecl` the redecl chain is not connected. So, we should 
pass `PrevDecl` then into `GetImportedOrCreateDecl`.  It would require to 
change all call expressions of `GetImportedOrCreateDecl` (58 occurences). 
Besides, we would use the `PrevDecl` only in case of inheritable attributes, 
only they require this special care. Note that there are a bunch of 
non-inheritable attributes which are already handled correctly in 
`InitializeImportedDecl`.




Comment at: clang/lib/AST/ASTImporter.cpp:7842
+// This implementation is inspired by Sema::mergeDeclAttributes.
+void ASTNodeImporter::CopyInheritedAttributes(FunctionDecl *Old,
+  FunctionDecl *New) {

shafik wrote:
> There are other attributes that [apply to functions as 
> well](https://en.cppreference.com/w/cpp/language/attributes): `nodiscard`, 
> `maybe_unused` and `deprecated`. Is there a reason not to support those as 
> well?
Different attributes require different handling.

E.g. C++11 [[noreturn]] requires diagnostics if the first declaration of a 
function does not specify a the attribute, but a later does:
```
void f();
[[noreturn]] void f(); // ERROR
```
If a function is declared with [[noreturn]] in one translation unit, and the 
same function is declared without [[noreturn]] in another translation unit, the 
program is ill-formed; no diagnostic required.

But this is not true to the GNU __attribute__((noreturn)).

Also, there are non-inheritable attributes, in their case we do not have to 
copy anything from the existing most-recent-decl.

Thus, I think I am going to change the patch's name to indicate we deal only 
with `__attribute__((noreturn))` and with `analyzer_noreturn`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68634



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

[PATCH] D68634: [ASTImporter] Imported FunctionDecls inherit attributes from existing functions

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D68634#1701192 , @martong wrote:

> In D68634#1700591 , @a_sidorin wrote:
>
> > Hello Balasz,
> >  In my opinion, importing and setting the attributes should be handled by 
> > the stuff used in InitializeImportedDecl(). Can we extend it or reuse the 
> > code? It will allow us not to miss the required actions while importing a 
> > new function too.
>
>
> I was thinking about that too, but it turned out to be a way more intrusive 
> change.
>
> We have to work with the most recent existing decl (`PrevDecl`) of the 
> FunctionDecl whose attribute we currently import.
>  We can get the `PrevDecl` only with the help of the lookup. We can't get it 
> from the `ToD` param of `InitializeImportedDecl` because by the time when we 
> call `InitializeImportedDecl` the redecl chain is not connected. So, we 
> should pass `PrevDecl` then into `GetImportedOrCreateDecl`.  It would require 
> to change all call expressions of `GetImportedOrCreateDecl` (58 occurences). 
> Besides, we would use the `PrevDecl` only in case of inheritable attributes, 
> only they require this special care. Note that there are a bunch of 
> non-inheritable attributes which are already handled correctly in 
> `InitializeImportedDecl`.


Actually, giving it some more thought, I realized it would make sense to pass 
the `PrevDecl` into `GetImportedOrCreateDecl` because that way we could connect 
the redecl chain right after the new node is created, and before importing any 
other node. That could facilitate structural eq check in the import of 
subsequent dependent nodes (see https://github.com/Ericsson/clang/issues/506).
Anyway, this change would be quite a big change compared to the original small 
patch we have here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68634



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

[PATCH] D68695: [clang-format] Update noexcept reference qualifiers detection

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
krasimir added reviewers: AndWass, MyDeveloperDay.

r373165 fixed an issue where a templated noexcept member function with a
reference qualifier would be indented more than expected:

  // Formatting produced with LLVM style with AlwaysBreakTemplateDeclarations: 
Yes
  
  // before r373165:
  struct f {
template 
void bar() && noexcept {}
  };
  
  // after:
  struct f {
template 
void bar() && noexcept {}
  };

The way this is done is that in the AnnotatingParser in
`lib/FormatTokenAnnotator.cpp` the determination of the usage of a `&` or `&&`
(the line in determineTokenType

  Current.Type = determineStarAmpUsage(...

is not performed in some cases anymore, combining with a few additional related
checks afterwards. The net effect of these checks results in the `&` or `&&`
token to start being classified as `TT_Unknown` in cases where before `r373165`
it would be classified as `TT_UnaryOperator` or `TT_PointerOrReference` by
`determineStarAmpUsage`.

This inadvertently caused 2 classes of regressions I'm aware of:

- The address-of `&` after a function assignment would be classified as 
`TT_Unknown`, causing spaces to surround it, disregarding style options:

  // before r373165:
  void (*fun_ptr)(void) = &fun;
  
  // after:
  void (*fun_ptr)(void) = & fun;



- In cases where there is a function declaration list -- looking macro between 
a template line and the start of the function declaration, an `&` as part of 
the return type would be classified as `TT_Unknown`, causing spaces to surround 
it:

  // before r373165:
  template 
  DEPRECATED("lala")
  Type& foo();
  
  // after:
  template 
  DEPRECATED("lala")
  Type & foo();

In these cases the problems are rooted in the skipping of the classification of
a `&` (and similarly `&&`) by determineStarAmpUsage which effects the formatting
decisions later in the pipeline.

I've looked into the goal of r373165 and noticed that replacing `noexcept` with
`const` in the given example produces no extra indentation with the old code:

  // before r373165:
  struct f {
template 
int foo() & const {}
  };
  
  struct f {
template 
int foo() & noexcept {}
  };

I investigated how clang-format annotated these two examples differently to
determine the places where the processing of both diverges in the pipeline.
There were two places where the processing diverges, causing the extra indent in
the `noexcept` case:

1. The `const` is annotated as a `TT_TrailingAnnotation`, whereas `noexcept` is 
annotated as `TT_Unknown`. I've updated the `determineTokenType` function to 
account for this by adding a missing `tok:kw_noexcept` to the clause that marks 
a token as `TT_TrailingAnnotation`.
2. The `&` in the second example is wrongly identified as `TT_BinaryOperator` 
in `determineStarAmpUsage`. This is the reason for the extra indentation -- 
clang-format gets confused and thinks this is an expression. I've updated 
`determineStarAmpUsage` to check for `tok:kw_noexcept`.

With these two updates in place, the additional parsing introduced by r373165
becomes unnecessary and all added tests pass (with updates, as now clang-format
respects the style configuration for spaces around the `&` in the test
examples).
I've removed these additions and added regression tests for the cases above.


Repository:
  rC Clang

https://reviews.llvm.org/D68695

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -7037,31 +7037,31 @@
 
   verifyFormat("struct f {\n"
"  template \n"
-   "  int &foo(const std::string &str) & noexcept {}\n"
+   "  int &foo(const std::string &str) &noexcept {}\n"
"};",
BreakTemplate);
 
   verifyFormat("struct f {\n"
"  template \n"
-   "  int &foo(const std::string &str) && noexcept {}\n"
+   "  int &foo(const std::string &str) &&noexcept {}\n"
"};",
BreakTemplate);
 
   verifyFormat("struct f {\n"
"  template \n"
-   "  int &foo(const std::string &str) const & noexcept {}\n"
+   "  int &foo(const std::string &str) const &noexcept {}\n"
"};",
BreakTemplate);
 
   verifyFormat("struct f {\n"
"  template \n"
-   "  int &foo(const std::string &str) const & noexcept {}\n"
+   "  int &foo(const std::string &str) const &noexcept {}\n"
"};",
BreakTemplate);
 
   verifyFormat("struct f {\n"
"  template \n"
-   "  auto foo(const std::string &str) && noexcept -> int & {}\n"
+   "  auto foo(const std::string &st

[PATCH] D68695: [clang-format] Update noexcept reference qualifiers detection

[Krasimir Georgiev via Phabricator via cfe-commits]

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



Comment at: unittests/Format/FormatTest.cpp:7093
"  template \n"
-   "  int& foo(const std::string& str) const & noexcept {}\n"
+   "  int& foo(const std::string& str) const&& noexcept {}\n"
"};",

note: the old test case was identical to the one above, I think the original 
intent was to check both `&` and `&&` cases, so modified accordingly.


Repository:
  rC Clang

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

https://reviews.llvm.org/D68695



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

[PATCH] D68695: [clang-format] Update noexcept reference qualifiers detection

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Only because I had problem finding the commit, this is as a consequence of 
D68072: [clang-format] Reference qualifiers in member templates causing extra 
indentation. 


Repository:
  rC Clang

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

https://reviews.llvm.org/D68695



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

[clang-tools-extra] r374163 - [clangd] Propagate context into reply handlers

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Wed Oct  9 05:48:41 2019
New Revision: 374163

URL: http://llvm.org/viewvc/llvm-project?rev=374163&view=rev
Log:
[clangd] Propagate context into reply handlers

Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.h

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.h?rev=374163&r1=374162&r2=374163&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h Wed Oct  9 05:48:41 2019
@@ -157,7 +157,7 @@ private:
   void call(StringRef Method, llvm::json::Value Params, Callback CB) 
{
 // Wrap the callback with LSP conversion and error-handling.
 auto HandleReply =
-[CB = std::move(CB)](
+[CB = std::move(CB), Ctx = Context::current().clone()](
 llvm::Expected RawResponse) mutable {
   Response Rsp;
   if (!RawResponse) {


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

[PATCH] D68695: [clang-format] Update noexcept reference qualifiers detection

[MyDeveloperDay via Phabricator via cfe-commits]

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

Thank you for fixing this, I'm sorry my review probably wasn't thorough enough 
in the first place, this LGTM and thank you for the very thorough description 
as this really helps me see.

On a side rant, for me this only adds fuel to my feeling that we need D68554: 
[clang-format] Proposal for clang-format to give compiler style warnings 
  and D29039: [clang-format] Proposal for 
clang-format -r option  in order to be able to 
run new clang-format binaries over a more substantial preformatted codebase 
quickly and easily, prior to commit of any changes.




Comment at: lib/Format/TokenAnnotator.cpp:1499
+} else if (Current.isOneOf(tok::identifier, tok::kw_const,
+   tok::kw_noexcept) &&
Current.Previous &&

I slightly wonder if other training annotations like `override` and `final` 
might also suffer simular to what we saw with {D68481}



Comment at: unittests/Format/FormatTest.cpp:7093
"  template \n"
-   "  int& foo(const std::string& str) const & noexcept {}\n"
+   "  int& foo(const std::string& str) const&& noexcept {}\n"
"};",

krasimir wrote:
> note: the old test case was identical to the one above, I think the original 
> intent was to check both `&` and `&&` cases, so modified accordingly.
nice catch


Repository:
  rC Clang

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

https://reviews.llvm.org/D68695



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

Re: [clang-tools-extra] r374163 - [clangd] Propagate context into reply handlers

[Nico Weber via cfe-commits]

Looks like this breaks three clangd tests on macOS:

Failing Tests (3):
Clangd :: code-action-request.test
Clangd :: execute-command.test
Clangd :: tweaks-format.test

libc++abi.dylib: terminating with uncaught exception of type
std::__1::system_error: mutex lock failed: Invalid argument

http://45.33.8.238/mac/1245/step_7.txt

On Wed, Oct 9, 2019 at 8:46 AM Kadir Cetinkaya via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: kadircet
> Date: Wed Oct  9 05:48:41 2019
> New Revision: 374163
>
> URL: http://llvm.org/viewvc/llvm-project?rev=374163&view=rev
> Log:
> [clangd] Propagate context into reply handlers
>
> Modified:
> clang-tools-extra/trunk/clangd/ClangdLSPServer.h
>
> Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.h?rev=374163&r1=374162&r2=374163&view=diff
>
> ==
> --- clang-tools-extra/trunk/clangd/ClangdLSPServer.h (original)
> +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h Wed Oct  9 05:48:41
> 2019
> @@ -157,7 +157,7 @@ private:
>void call(StringRef Method, llvm::json::Value Params,
> Callback CB) {
>  // Wrap the callback with LSP conversion and error-handling.
>  auto HandleReply =
> -[CB = std::move(CB)](
> +[CB = std::move(CB), Ctx = Context::current().clone()](
>  llvm::Expected RawResponse) mutable {
>Response Rsp;
>if (!RawResponse) {
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68695: [clang-format] Update noexcept reference qualifiers detection

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir marked 3 inline comments as done.
krasimir added inline comments.



Comment at: lib/Format/TokenAnnotator.cpp:1499
+} else if (Current.isOneOf(tok::identifier, tok::kw_const,
+   tok::kw_noexcept) &&
Current.Previous &&

MyDeveloperDay wrote:
> I slightly wonder if other training annotations like `override` and `final` 
> might also suffer simular to what we saw with {D68481}
I'll check these cases and add appropriate tests etc. in a subsequent patch 
(maybe next week).

I think that part of the issue is that the trailing annotations back in the day 
where this code was written (7 years ago judging by a few blames) were captured 
by the `tok::identifier` case; however since then `C++` and `clang` have 
evolved a lot and have extracted separate tokens for these, hence the missing 
cases.


Repository:
  rC Clang

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

https://reviews.llvm.org/D68695



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

[PATCH] D67160: [clang, ARM] Default to -fno-lax-vector-conversions in ARM v8.1-M.

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham updated this revision to Diff 224031.
simon_tatham added a comment.

Rebased to current master.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67160

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/lax-vector-conversions.c


Index: clang/test/Driver/lax-vector-conversions.c
===
--- /dev/null
+++ clang/test/Driver/lax-vector-conversions.c
@@ -0,0 +1,8 @@
+// RUN: %clang --target=arm-arm-none-eabi -march=armv8.1m.main -### -c %s 2>&1 
\
+// RUN:   | FileCheck --check-prefix=CHECK-STRICT %s
+
+// RUN: %clang --target=arm-arm-none-eabi -march=armv8a -### -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-LAX %s
+
+// CHECK-STRICT: "-flax-vector-conversions=none"
+// CHECK-LAX-NOT: "-flax-vector-conversions=none"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -1377,6 +1377,17 @@
   }
 }
 
+static bool isLaxVectorConversionsDefault(const llvm::Triple &Triple) {
+  switch (Triple.getArch()) {
+  default:
+return true;
+
+  case llvm::Triple::thumb:
+  case llvm::Triple::thumbeb:
+return Triple.getSubArch() != llvm::Triple::ARMSubArch_v8_1m_mainline;
+  }
+}
+
 static bool isNoCommonDefault(const llvm::Triple &Triple) {
   switch (Triple.getArch()) {
   default:
@@ -4678,7 +4689,11 @@
   (Args.hasArg(options::OPT_mkernel) && types::isCXX(InputType)))
 CmdArgs.push_back("-fapple-kext");
 
-  Args.AddLastArg(CmdArgs, options::OPT_flax_vector_conversions_EQ);
+  if (Args.getLastArg(options::OPT_flax_vector_conversions_EQ)) {
+Args.AddLastArg(CmdArgs, options::OPT_flax_vector_conversions_EQ);
+  } else if (!isLaxVectorConversionsDefault(Triple)) {
+CmdArgs.push_back("-flax-vector-conversions=none");
+  }
   Args.AddLastArg(CmdArgs, options::OPT_fobjc_sender_dependent_dispatch);
   Args.AddLastArg(CmdArgs, options::OPT_fdiagnostics_print_source_range_info);
   Args.AddLastArg(CmdArgs, options::OPT_fdiagnostics_parseable_fixits);


Index: clang/test/Driver/lax-vector-conversions.c
===
--- /dev/null
+++ clang/test/Driver/lax-vector-conversions.c
@@ -0,0 +1,8 @@
+// RUN: %clang --target=arm-arm-none-eabi -march=armv8.1m.main -### -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-STRICT %s
+
+// RUN: %clang --target=arm-arm-none-eabi -march=armv8a -### -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-LAX %s
+
+// CHECK-STRICT: "-flax-vector-conversions=none"
+// CHECK-LAX-NOT: "-flax-vector-conversions=none"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -1377,6 +1377,17 @@
   }
 }
 
+static bool isLaxVectorConversionsDefault(const llvm::Triple &Triple) {
+  switch (Triple.getArch()) {
+  default:
+return true;
+
+  case llvm::Triple::thumb:
+  case llvm::Triple::thumbeb:
+return Triple.getSubArch() != llvm::Triple::ARMSubArch_v8_1m_mainline;
+  }
+}
+
 static bool isNoCommonDefault(const llvm::Triple &Triple) {
   switch (Triple.getArch()) {
   default:
@@ -4678,7 +4689,11 @@
   (Args.hasArg(options::OPT_mkernel) && types::isCXX(InputType)))
 CmdArgs.push_back("-fapple-kext");
 
-  Args.AddLastArg(CmdArgs, options::OPT_flax_vector_conversions_EQ);
+  if (Args.getLastArg(options::OPT_flax_vector_conversions_EQ)) {
+Args.AddLastArg(CmdArgs, options::OPT_flax_vector_conversions_EQ);
+  } else if (!isLaxVectorConversionsDefault(Triple)) {
+CmdArgs.push_back("-flax-vector-conversions=none");
+  }
   Args.AddLastArg(CmdArgs, options::OPT_fobjc_sender_dependent_dispatch);
   Args.AddLastArg(CmdArgs, options::OPT_fdiagnostics_print_source_range_info);
   Args.AddLastArg(CmdArgs, options::OPT_fdiagnostics_parseable_fixits);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67159: [clang] New __attribute__((__clang_arm_mve_alias)).

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham updated this revision to Diff 224030.
simon_tatham added a comment.

Rebased to current master.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67159

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/Decl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/arm-mve-alias-attribute.c

Index: clang/test/Sema/arm-mve-alias-attribute.c
===
--- /dev/null
+++ clang/test/Sema/arm-mve-alias-attribute.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple armv8.1m.main-arm-none-eabi -verify -fsyntax-only %s
+
+static __inline__ __attribute__((__clang_arm_mve_alias(__builtin_arm_nop))) // expected-error {{'__clang_arm_mve_alias' attribute can only be applied to an ARM MVE builtin}}
+void nop(void);
+
+static __inline__ __attribute__((__clang_arm_mve_alias)) // expected-error {{'__clang_arm_mve_alias' attribute takes one argument}}
+void noparens(void);
+
+static __inline__ __attribute__((__clang_arm_mve_alias())) // expected-error {{'__clang_arm_mve_alias' attribute takes one argument}}
+void emptyparens(void);
+
+static __inline__ __attribute__((__clang_arm_mve_alias("string literal"))) // expected-error {{'__clang_arm_mve_alias' attribute requires parameter 1 to be an identifier}}
+void stringliteral(void);
+
+static __inline__ __attribute__((__clang_arm_mve_alias(1))) // expected-error {{'__clang_arm_mve_alias' attribute requires parameter 1 to be an identifier}}
+void integer(void);
+
+static __inline__ __attribute__((__clang_arm_mve_alias(__builtin_arm_nop, 2))) // expected-error {{'__clang_arm_mve_alias' attribute takes one argument}}
+void twoargs(void);
+
+static __attribute__((__clang_arm_mve_alias(__builtin_arm_nop))) // expected-error {{'__clang_arm_mve_alias' attribute only applies to functions}}
+int variable;
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
@@ -17,6 +17,7 @@
 // CHECK-NEXT: Annotate ()
 // CHECK-NEXT: AnyX86NoCfCheck (SubjectMatchRule_hasType_functionType)
 // CHECK-NEXT: ArcWeakrefUnavailable (SubjectMatchRule_objc_interface)
+// CHECK-NEXT: ArmMveAlias (SubjectMatchRule_function)
 // CHECK-NEXT: AssumeAligned (SubjectMatchRule_objc_method, SubjectMatchRule_function)
 // CHECK-NEXT: Availability ((SubjectMatchRule_record, SubjectMatchRule_enum, SubjectMatchRule_enum_constant, SubjectMatchRule_field, SubjectMatchRule_function, SubjectMatchRule_namespace, SubjectMatchRule_objc_category, SubjectMatchRule_objc_implementation, SubjectMatchRule_objc_interface, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property, SubjectMatchRule_objc_protocol, SubjectMatchRule_record, SubjectMatchRule_type_alias, SubjectMatchRule_variable))
 // CHECK-NEXT: CFAuditedTransfer (SubjectMatchRule_function)
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -23,6 +23,7 @@
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TargetBuiltins.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Sema/DeclSpec.h"
@@ -4830,6 +4831,33 @@
  XRayLogArgsAttr(S.Context, AL, ArgCount.getSourceIndex()));
 }
 
+static bool ArmMveAliasValid(unsigned BuiltinID, StringRef AliasName) {
+  // FIXME: this will be filled in by Tablegen which isn't written yet
+  return false;
+}
+
+static void handleArmMveAliasAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+  if (!checkAttributeNumArgs(S, AL, 1))
+return;
+
+  if (!AL.isArgIdent(0)) {
+S.Diag(AL.getLoc(), diag::err_attribute_argument_n_type)
+<< AL << 1 << AANT_ArgumentIdentifier;
+return;
+  }
+
+  IdentifierInfo *Ident = AL.getArgAsIdent(0)->Ident;
+  unsigned BuiltinID = Ident->getBuiltinID();
+
+  if (!ArmMveAliasValid(BuiltinID,
+cast(D)->getIdentifier()->getName())) {
+S.Diag(AL.getLoc(), diag::err_attribute_arm_mve_alias);
+return;
+  }
+
+  D->addAttr(::new (S.Context) ArmMveAliasAttr(S.Context, AL, Ident));
+}
+
 //===--===//
 // Checker-specific attribute handlers.
 //===--===//
@@ -7160,6 +7188,10 @@
   case ParsedAttr::AT_MSAllocator:
 handleMSAllocatorAttr(S, D, AL);
 break;
+
+  case ParsedAttr::AT_ArmMveAlias

[PATCH] D68701: Adds a isDefinitionKind to FunctionDecl

[Guillaume Chatelet via Phabricator via cfe-commits]

gchatelet created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Tentatively fixing the bug described in 
https://reviews.llvm.org/D68028#inline-614831.
This is not a good solution though since `isThisDeclarationADefinition` still 
doesn't work as expected.

Having `isThisDeclarationADefinition` use `isDefinitionKind` is breaking a lot 
of tests.
I believe there is broken code out there relying on the current behavior.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68701

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclBase.h
  clang/lib/AST/Decl.cpp
  clang/lib/Sema/SemaDecl.cpp

Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -8565,6 +8565,26 @@
   isVirtualOkay);
   if (!NewFD) return nullptr;
 
+  // If a function is defined as defaulted or deleted, mark it as such now.
+  // FIXME: Does this ever happen? ActOnStartOfFunctionDef forces the function
+  // definition kind to FDK_Definition.
+  switch (D.getFunctionDefinitionKind()) {
+case FDK_Declaration:
+  break;
+
+case FDK_Definition:
+  NewFD->setDefinitionKind();
+  break;
+
+case FDK_Defaulted:
+  NewFD->setDefaulted();
+  break;
+
+case FDK_Deleted:
+  NewFD->setDeletedAsWritten();
+  break;
+  }
+
   if (OriginalLexicalContext && OriginalLexicalContext->isObjCContainer())
 NewFD->setTopLevelDeclInObjCContainer();
 
@@ -8817,23 +8837,6 @@
   NewFD->setAccess(AS_public);
 }
 
-// If a function is defined as defaulted or deleted, mark it as such now.
-// FIXME: Does this ever happen? ActOnStartOfFunctionDef forces the function
-// definition kind to FDK_Definition.
-switch (D.getFunctionDefinitionKind()) {
-  case FDK_Declaration:
-  case FDK_Definition:
-break;
-
-  case FDK_Defaulted:
-NewFD->setDefaulted();
-break;
-
-  case FDK_Deleted:
-NewFD->setDeletedAsWritten();
-break;
-}
-
 if (isa(NewFD) && DC == CurContext &&
 D.isFunctionDefinition()) {
   // C++ [class.mfct]p2:
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -2792,6 +2792,7 @@
   FunctionDeclBits.IsMultiVersion = false;
   FunctionDeclBits.IsCopyDeductionCandidate = false;
   FunctionDeclBits.HasODRHash = false;
+  FunctionDeclBits.IsDefinitionKind = false;
 }
 
 void FunctionDecl::getNameForDiagnostic(
Index: clang/include/clang/AST/DeclBase.h
===
--- clang/include/clang/AST/DeclBase.h
+++ clang/include/clang/AST/DeclBase.h
@@ -1534,10 +1534,14 @@
 
 /// Store the ODRHash after first calculation.
 uint64_t HasODRHash : 1;
+
+/// Indicates that this function has the FDK_Definition
+/// FunctionDefinitionKind.
+uint64_t IsDefinitionKind : 1;
   };
 
   /// Number of non-inherited bits in FunctionDeclBitfields.
-  enum { NumFunctionDeclBits = 25 };
+  enum { NumFunctionDeclBits = 26 };
 
   /// Stores the bits used by CXXConstructorDecl. If modified
   /// NumCXXConstructorDeclBits and the accessor
@@ -1549,12 +1553,12 @@
 /// For the bits in FunctionDeclBitfields.
 uint64_t : NumFunctionDeclBits;
 
-/// 24 bits to fit in the remaining available space.
+/// 25 bits to fit in the remaining available space.
 /// Note that this makes CXXConstructorDeclBitfields take
 /// exactly 64 bits and thus the width of NumCtorInitializers
 /// will need to be shrunk if some bit is added to NumDeclContextBitfields,
 /// NumFunctionDeclBitfields or CXXConstructorDeclBitfields.
-uint64_t NumCtorInitializers : 23;
+uint64_t NumCtorInitializers : 22;
 uint64_t IsInheritingConstructor : 1;
 
 /// Whether this constructor has a trail-allocated explicit specifier.
Index: clang/include/clang/AST/Decl.h
===
--- clang/include/clang/AST/Decl.h
+++ clang/include/clang/AST/Decl.h
@@ -2284,6 +2284,12 @@
   bool willHaveBody() const { return FunctionDeclBits.WillHaveBody; }
   void setWillHaveBody(bool V = true) { FunctionDeclBits.WillHaveBody = V; }
 
+  /// True if this function is of type of Kind FDK_Definition.
+  bool isDefinitionKind() const { return FunctionDeclBits.IsDefinitionKind; }
+  void setDefinitionKind(bool V = true) {
+FunctionDeclBits.IsDefinitionKind = V;
+  }
+
   /// True if this function is considered a multiversioned function.
   bool isMultiVersion() const {
 return getCanonicalDecl()->FunctionDeclBits.IsMultiVersion;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68028: [clang] Add no_builtin attribute

[Guillaume Chatelet via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:1098-1099
+
+  if (D->hasAttr())
+D->dropAttr();
+

gchatelet wrote:
> aaron.ballman wrote:
> > gchatelet wrote:
> > > gchatelet wrote:
> > > > aaron.ballman wrote:
> > > > > Just making sure I understand the semantics you want: redeclarations 
> > > > > do not have to have matching lists of builtins, but instead the 
> > > > > definition will use a merged list? e.g.,
> > > > > ```
> > > > > [[clang::no_builtin("memset")]] void whatever();
> > > > > [[clang::no_builtin("memcpy")]] void whatever();
> > > > > 
> > > > > [[clang::no_builtin("memmove")]] void whatever() {
> > > > >  // Will not use memset, memcpy, or memmove builtins.
> > > > > }
> > > > > ```
> > > > > That seems a bit strange, to me. In fact, being able to apply this 
> > > > > attribute to a declaration seems a bit like a mistake -- this only 
> > > > > impacts the definition of the function, and I can't imagine good 
> > > > > things coming from hypothetical code like:
> > > > > ```
> > > > > [[clang::no_builtin("memset")]] void whatever();
> > > > > #include "whatever.h" // Provides a library declaration of whatever() 
> > > > > with no restrictions on it
> > > > > ```
> > > > > WDYT about restricting this attribute to only appear on definitions?
> > > > That's a very good point. Thx for noticing.
> > > > Indeed I think it only makes sense to have the attribute on the 
> > > > function definition.
> > > > 
> > > > I've tried to to use `FunctionDecl->hasBody()` during attribute 
> > > > handling in the Sema phase but it seems like the `FunctionDecl` is not 
> > > > complete at this point.
> > > > All calls to `hasBody()` return `false`, if I repeat the operation in 
> > > > `CGCall` then `hasBody` returns `true` and I can see the 
> > > > `CompoundStatement`.
> > > > 
> > > > Do you have any recommendations on where to perform the check?
> > > So after some investigations it turns out that 
> > > `FunctionDecl::isThisDeclarationADefinition` is buggy (returns always 
> > > `false`) when called from `ProcessDeclAttribute`.
> > > I believe it's because the `CompoundStatement` is not parsed at this 
> > > point.
> > > 
> > > As a matter of fact all code using this function in 
> > > `ProcessDeclAttribute` is dead code (see D68379 which disables the dead 
> > > code, tests are still passing)
> > > 
> > > 
> > > I'm unsure of how to fix this, it may be possible of using 
> > > `FunctionDecl::setWillHaveBody`in [[ 
> > > https://github.com/llvm/llvm-project/blob/0577a0cedbc5be4cd4c20ba53d3dbdac6bff9a0a/clang/lib/Sema/SemaDecl.cpp#L8820
> > >  | this switch ]].
> > > So after some investigations it turns out that 
> > > FunctionDecl::isThisDeclarationADefinition is buggy (returns always 
> > > false) when called from ProcessDeclAttribute.
> > 
> > That is a bit odd to me; we call it in a half dozen places in 
> > SemaDeclAttr.cpp, all of which get called from `ProcessDeclAttribute`. Are 
> > ALL of those uses broken currently?!
> > 
> > > As a matter of fact all code using this function in ProcessDeclAttribute 
> > > is dead code (see D68379 which disables the dead code, tests are still 
> > > passing)
> > 
> > You got four of the six. What about the use in 
> > `handleObjCSuppresProtocolAttr()` and the second use in `handleAliasAttr()`?
> > 
> > > I'm unsure of how to fix this, it may be possible of using 
> > > FunctionDecl::setWillHaveBodyin this switch.
> > 
> > I'm also unsure of a good way forward. @rsmith may have ideas too.
> > That is a bit odd to me; we call it in a half dozen places in 
> > SemaDeclAttr.cpp, all of which get called from ProcessDeclAttribute. Are 
> > ALL of those uses broken currently?!
> > You got four of the six. What about the use in 
> > handleObjCSuppresProtocolAttr() and the second use in handleAliasAttr()?
> 
> It really is `FunctionDecl::isThisDeclarationADefinition` that is buggy, the 
> other places where we call `isThisDeclarationADefinition` in 
> `SemaDeclAttr.cpp` are ok, i.e. `VarDecl::isThisDeclarationADefinition` and 
> `ObjCProtocolDecl::isThisDeclarationADefinition`
> 
> I tried to use `FunctionDecl::setWillHaveBody`but it broke many tests so it 
> seems inappropriate.
> 
I've tried to fix it in D68701. I'm pretty sure this is not the right way to do 
it though,
@rsmith any idea on how to proceed?

In the meantime I'll add a FIXME and move on with this patch if you don't mind.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68028



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

[PATCH] D67901: [clangd] Improve semantic highlighting in dependent contexts (fixes #154)

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:219
+  bool VisitDependentNameTypeLoc(DependentNameTypeLoc L) {
+addToken(L.getNameLoc(), HighlightingKind::DependentType);
+return true;

nridge wrote:
> ilya-biryukov wrote:
> > nridge wrote:
> > > hokein wrote:
> > > > nit: we have `kindForType` for hanlding all types, so I'd move the 
> > > > logic of detecting the dependent type there.
> > > I did try this, but it doesn't quite work, because `VisitTypeLoc` adds 
> > > the highlighting to the `TypeLoc`'s `getBeginLoc()`. For something like 
> > > `typename T::type`, that highlights the `typename` token rather than the 
> > > `type` token. By contrast, here I add the highlighting to the 
> > > `DependentNameTypeLoc`'s `getNameLoc()` which will correctly highlight 
> > > the `type` token.
> > You'd want to implement `WalkUpFromDependentNameTypeLoc` instead of 
> > `Visit*` to avoid adding extra highlightings in `VisitTypeLoc`.
> > 
> > In fact, I'm surprised we're not seeing them now.
> We're not seeing extra highlightings with the current patch, because 
> `VisitTypeLoc` does not add any highlightings for dependent types 
> (`kindForType()` returns `None` for them). 
> 
> So, I don't think there's a problem with using `VisitDependentNameTypeLoc`?
I'm second on Ilya's suggestion. It follows the existing pattern (see  
WalkupFrom* methods above) and make the code clearer, and we can move the 
`DependentType` to `kindForType`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67901



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

[PATCH] D67160: [clang, ARM] Default to -fno-lax-vector-conversions in ARM v8.1-M.

[Kristof Beyls via Phabricator via cfe-commits]

kristof.beyls added a comment.

Would it still make sense to have this patch after D68683 
 lands? At first sight, it seems this patch 
might no longer make sense then?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67160



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

[PATCH] D68695: [clang-format] Update noexcept reference qualifiers detection

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: lib/Format/TokenAnnotator.cpp:1499
+} else if (Current.isOneOf(tok::identifier, tok::kw_const,
+   tok::kw_noexcept) &&
Current.Previous &&

krasimir wrote:
> MyDeveloperDay wrote:
> > I slightly wonder if other training annotations like `override` and `final` 
> > might also suffer simular to what we saw with {D68481}
> I'll check these cases and add appropriate tests etc. in a subsequent patch 
> (maybe next week).
> 
> I think that part of the issue is that the trailing annotations back in the 
> day where this code was written (7 years ago judging by a few blames) were 
> captured by the `tok::identifier` case; however since then `C++` and `clang` 
> have evolved a lot and have extracted separate tokens for these, hence the 
> missing cases.
Sound good..


Repository:
  rC Clang

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

https://reviews.llvm.org/D68695



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

Re: [clang-tools-extra] r374163 - [clangd] Propagate context into reply handlers

[Kadir Çetinkaya via cfe-commits]

thanks! sent out D68702.

On Wed, Oct 9, 2019 at 2:56 PM Nico Weber  wrote:

> Looks like this breaks three clangd tests on macOS:
>
> Failing Tests (3):
> Clangd :: code-action-request.test
> Clangd :: execute-command.test
> Clangd :: tweaks-format.test
>
> libc++abi.dylib: terminating with uncaught exception of type
> std::__1::system_error: mutex lock failed: Invalid argument
>
> http://45.33.8.238/mac/1245/step_7.txt
>
> On Wed, Oct 9, 2019 at 8:46 AM Kadir Cetinkaya via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: kadircet
>> Date: Wed Oct  9 05:48:41 2019
>> New Revision: 374163
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=374163&view=rev
>> Log:
>> [clangd] Propagate context into reply handlers
>>
>> Modified:
>> clang-tools-extra/trunk/clangd/ClangdLSPServer.h
>>
>> Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.h?rev=374163&r1=374162&r2=374163&view=diff
>>
>> ==
>> --- clang-tools-extra/trunk/clangd/ClangdLSPServer.h (original)
>> +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h Wed Oct  9 05:48:41
>> 2019
>> @@ -157,7 +157,7 @@ private:
>>void call(StringRef Method, llvm::json::Value Params,
>> Callback CB) {
>>  // Wrap the callback with LSP conversion and error-handling.
>>  auto HandleReply =
>> -[CB = std::move(CB)](
>> +[CB = std::move(CB), Ctx = Context::current().clone()](
>>  llvm::Expected RawResponse) mutable {
>>Response Rsp;
>>if (!RawResponse) {
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68702: [clangd] Make sure ReplyCallbacks are destroyed before RequestCancelersMutex

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

After rL374163 , replycallbacks might have a 
cancellable context, which
will try to access RequestCancellers on destruction. See
http://45.33.8.238/mac/1245/step_7.txt for a sample failure.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68702

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp


Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -371,16 +371,6 @@
 
   llvm::StringMap> Notifications;
   llvm::StringMap> Calls;
-  // The maximum number of callbacks held in clangd.
-  //
-  // We bound the maximum size to the pending map to prevent memory leakage
-  // for cases where LSP clients don't reply for the request.
-  static constexpr int MaxReplayCallbacks = 100;
-  mutable std::mutex CallMutex;
-  int NextCallID = 0; /* GUARDED_BY(CallMutex) */
-  std::deque>>
-  ReplyCallbacks; /* GUARDED_BY(CallMutex) */
 
   // Method calls may be cancelled by ID, so keep track of their state.
   // This needs a mutex: handlers may finish on a different thread, and that's
@@ -432,6 +422,17 @@
 }));
   }
 
+  // The maximum number of callbacks held in clangd.
+  //
+  // We bound the maximum size to the pending map to prevent memory leakage
+  // for cases where LSP clients don't reply for the request.
+  static constexpr int MaxReplayCallbacks = 100;
+  mutable std::mutex CallMutex;
+  int NextCallID = 0; /* GUARDED_BY(CallMutex) */
+  std::deque>>
+  ReplyCallbacks; /* GUARDED_BY(CallMutex) */
+
   ClangdLSPServer &Server;
 };
 constexpr int ClangdLSPServer::MessageHandler::MaxReplayCallbacks;


Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -371,16 +371,6 @@
 
   llvm::StringMap> Notifications;
   llvm::StringMap> Calls;
-  // The maximum number of callbacks held in clangd.
-  //
-  // We bound the maximum size to the pending map to prevent memory leakage
-  // for cases where LSP clients don't reply for the request.
-  static constexpr int MaxReplayCallbacks = 100;
-  mutable std::mutex CallMutex;
-  int NextCallID = 0; /* GUARDED_BY(CallMutex) */
-  std::deque>>
-  ReplyCallbacks; /* GUARDED_BY(CallMutex) */
 
   // Method calls may be cancelled by ID, so keep track of their state.
   // This needs a mutex: handlers may finish on a different thread, and that's
@@ -432,6 +422,17 @@
 }));
   }
 
+  // The maximum number of callbacks held in clangd.
+  //
+  // We bound the maximum size to the pending map to prevent memory leakage
+  // for cases where LSP clients don't reply for the request.
+  static constexpr int MaxReplayCallbacks = 100;
+  mutable std::mutex CallMutex;
+  int NextCallID = 0; /* GUARDED_BY(CallMutex) */
+  std::deque>>
+  ReplyCallbacks; /* GUARDED_BY(CallMutex) */
+
   ClangdLSPServer &Server;
 };
 constexpr int ClangdLSPServer::MessageHandler::MaxReplayCallbacks;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68562: [clangd] Add RemoveUsingNamespace tweak.

[UTKARSH SAXENA via Phabricator via cfe-commits]

usaxena95 added inline comments.



Comment at: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp:87
+return false;
+  if (const Decl *ParentDecl = Node->Parent->ASTNode.get())
+return llvm::isa(ParentDecl);

ilya-biryukov wrote:
> Can we use `ASTNode.get()` to directly to check for this?
> 
> Not sure how `DynTypedNode` works, though, maybe that's impossible.
Works for the test. 
The doc for `get` says that it returns NULL if the stored node does not have 
a type that is **convertible** to `T`.



Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:782
+  int main() {
+aa::map m;
+  }

ilya-biryukov wrote:
> This is incorrect, right? We should not be qualifying here.
> See the relevant comment on `isInsideNamespace`
Works fine now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68562



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

[PATCH] D68562: [clangd] Add RemoveUsingNamespace tweak.

[UTKARSH SAXENA via Phabricator via cfe-commits]

usaxena95 updated this revision to Diff 224038.
usaxena95 marked 5 inline comments as done.
usaxena95 added a comment.

Make action unavailable if the namespace contains a using namespace decl.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68562

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/AST.h
  clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
  clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -69,7 +69,8 @@
   EXPECT_EQ(apply("^if (true) {return 100;} else {continue;}"),
 "if (true) {continue;} else {return 100;}");
   EXPECT_EQ(apply("^if () {return 100;} else {continue;}"),
-"if () {continue;} else {return 100;}") << "broken condition";
+"if () {continue;} else {return 100;}")
+  << "broken condition";
   EXPECT_AVAILABLE("^i^f^^(^t^r^u^e^) { return 100; } ^e^l^s^e^ { continue; }");
   EXPECT_UNAVAILABLE("if (true) {^return ^100;^ } else { ^continue^;^ }");
   // Available in subexpressions of the condition;
@@ -100,7 +101,7 @@
   EXPECT_UNAVAILABLE(R"cpp(R"(multi )" ^"token " u8"str\ning")cpp"); // nonascii
   EXPECT_UNAVAILABLE(R"cpp(^R^"^(^multi )" "token " "str\ning")cpp"); // raw
   EXPECT_UNAVAILABLE(R"cpp(^"token\n" __FILE__)cpp"); // chunk is macro
-  EXPECT_UNAVAILABLE(R"cpp(^"a\r\n";)cpp"); // forbidden escape char
+  EXPECT_UNAVAILABLE(R"cpp(^"a\r\n";)cpp");   // forbidden escape char
 
   const char *Input = R"cpp(R"(multi
 token)" "\nst^ring\n" "literal")cpp";
@@ -286,11 +287,11 @@
  void f(int a) {
int y = PLUS([[1+a]]);
  })cpp",
-  /*FIXME: It should be extracted like this.
-   R"cpp(#define PLUS(x) x++
- void f(int a) {
-   auto dummy = 1+a; int y = PLUS(dummy);
- })cpp"},*/
+   /*FIXME: It should be extracted like this.
+R"cpp(#define PLUS(x) x++
+  void f(int a) {
+auto dummy = 1+a; int y = PLUS(dummy);
+  })cpp"},*/
R"cpp(#define PLUS(x) x++
  void f(int a) {
auto dummy = PLUS(1+a); int y = dummy;
@@ -301,13 +302,13 @@
if(1)
 LOOP(5 + [[3]])
  })cpp",
-  /*FIXME: It should be extracted like this. SelectionTree needs to be
-* fixed for macros.
-   R"cpp(#define LOOP(x) while (1) {a = x;}
-   void f(int a) {
- auto dummy = 3; if(1)
-  LOOP(5 + dummy)
-   })cpp"},*/
+   /*FIXME: It should be extracted like this. SelectionTree needs to be
+ * fixed for macros.
+R"cpp(#define LOOP(x) while (1) {a = x;}
+void f(int a) {
+  auto dummy = 3; if(1)
+   LOOP(5 + dummy)
+})cpp"},*/
R"cpp(#define LOOP(x) while (1) {a = x;}
  void f(int a) {
auto dummy = LOOP(5 + 3); if(1)
@@ -403,8 +404,8 @@
  void f() {
auto dummy = S(2) + S(3) + S(4); S x = S(1) + dummy + S(5);
  })cpp"},
-   // Don't try to analyze across macro boundaries
-   // FIXME: it'd be nice to do this someday (in a safe way)
+  // Don't try to analyze across macro boundaries
+  // FIXME: it'd be nice to do this someday (in a safe way)
   {R"cpp(#define ECHO(X) X
  void f() {
int x = 1 + [[ECHO(2 + 3) + 4]] + 5;
@@ -521,7 +522,7 @@
   StartsWith("fail: Could not expand type of lambda expression"));
   // inline namespaces
   EXPECT_EQ(apply("au^to x = inl_ns::Visible();"),
-  "Visible x = inl_ns::Visible();");
+"Visible x = inl_ns::Visible();");
   // local class
   EXPECT_EQ(apply("namespace x { void y() { struct S{}; ^auto z = S(); } }"),
 "namespace x { void y() { struct S{}; S z = S(); } }");
@@ -656,6 +657,208 @@
   EXPECT_THAT(apply(" for(;;) { [[while(1) break; break;]] }"),
   StartsWith("fail"));
 }
+
+TWEAK_TEST(RemoveUsingNamespace);
+TEST_F(RemoveUsingNamespaceTest, All) {
+  std::pair Cases[] = {
+  {// Remove all occurrences of ns. Qualify only unqualified.
+   R"cpp(
+  namespace ns1 { struct vector {}; }
+  namespace ns2 { struct map {}; }
+  using namespace n^s1;
+  using namespace ns2;
+  using namespace ns1;
+  int main() {
+ns1::vector v1;
+vector v2;
+map m1;
+  }
+)cpp",
+   R"cpp(
+ 

[PATCH] D68660: [tooling] Teach Tooling to understand compilation with offloading.

[Michael Liao via Phabricator via cfe-commits]

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



Comment at: clang/lib/Tooling/Tooling.cpp:117
   // The one job we find should be to invoke clang again.
   const auto &Cmd = cast(*Jobs.begin());
   if (StringRef(Cmd.getCreator().getName()) != "clang") {

tra wrote:
> Is this still the right job for us to pick? I think we want this to be the 
> host compilation. 
> 
> As things are right now my guess is that we're probably picking the first 
> device-side compilation.
yes, OffloadAction is appended to the original host action. The corresponding 
first job is the host compilation. The frontend has similar handling in 
`lib/Frontend/CreateInvocationFromCommandLine.cpp`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68660



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

[PATCH] D68702: [clangd] Make sure ReplyCallbacks are destroyed before RequestCancelersMutex

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:429
+  // for cases where LSP clients don't reply for the request.
+  static constexpr int MaxReplayCallbacks = 100;
+  mutable std::mutex CallMutex;

Please document that this has to go after `RequestCancelersMutex`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68702



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

[PATCH] D67160: [clang, ARM] Default to -fno-lax-vector-conversions in ARM v8.1-M.

[Dave Green via Phabricator via cfe-commits]

dmgreen added a comment.

Yes.. My understanding is that the default is still 
-flax-vector-convertions=all (the old clang behaviour), but the plan is to 
change it (for all architectures) to none.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67160



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

[PATCH] D68702: [clangd] Make sure ReplyCallbacks are destroyed before RequestCancelersMutex

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Maybe inline the error from the bot. Right now stuff on 45.33.8.238 is very in 
flux.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68702



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

[PATCH] D68681: [libc++][test] Miscellaneous MSVC cleanups

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

Quuxplusone added inline comments.



Comment at: 
test/std/containers/unord/unord.map/unord.map.cnstr/deduct.pass.cpp:70
+#pragma warning(disable: 4244) // '%s': conversion from '%s' to '%s', possible 
loss of data
+#endif // TEST_COMPILER_C1XX
+

Alternatively, if you wanted to change the instances of `hash` to 
`hash<>`, that would be fine with me. (Just as long as it's something 
distinguishable from the default of `hash`.)


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

https://reviews.llvm.org/D68681



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

r374167 - [clang-offload-bundler] Support `.cui` and `.d`.

[Michael Liao via cfe-commits]

Author: hliao
Date: Wed Oct  9 06:53:37 2019
New Revision: 374167

URL: http://llvm.org/viewvc/llvm-project?rev=374167&view=rev
Log:
[clang-offload-bundler] Support `.cui` and `.d`.

Reviewers: tra, yaxunl

Subscribers: cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp

Modified: cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp?rev=374167&r1=374166&r2=374167&view=diff
==
--- cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp (original)
+++ cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp Wed Oct  9 
06:53:37 2019
@@ -71,6 +71,8 @@ static cl::opt
"Current supported types are:\n"
"  i   - cpp-output\n"
"  ii  - c++-cpp-output\n"
+   "  cui - cuda/hip-output\n"
+   "  d   - dependency\n"
"  ll  - llvm\n"
"  bc  - llvm-bc\n"
"  s   - assembler\n"
@@ -628,6 +630,12 @@ static FileHandler *CreateFileHandler(Me
 return new TextFileHandler(/*Comment=*/"//");
   if (FilesType == "ii")
 return new TextFileHandler(/*Comment=*/"//");
+  if (FilesType == "cui")
+return new TextFileHandler(/*Comment=*/"//");
+  // TODO: `.d` should be eventually removed once `-M` and its variants are
+  // handled properly in offload compilation.
+  if (FilesType == "d")
+return new TextFileHandler(/*Comment=*/"#");
   if (FilesType == "ll")
 return new TextFileHandler(/*Comment=*/";");
   if (FilesType == "bc")


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

[PATCH] D68663: [clang-offload-bundler] Support `.cui` and `.d`.

[Michael Liao via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2ae54aba03f2: [clang-offload-bundler] Support `.cui` and 
`.d`. (authored by hliao).

Changed prior to commit:
  https://reviews.llvm.org/D68663?vs=223932&id=224044#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68663

Files:
  clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp


Index: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -71,6 +71,8 @@
"Current supported types are:\n"
"  i   - cpp-output\n"
"  ii  - c++-cpp-output\n"
+   "  cui - cuda/hip-output\n"
+   "  d   - dependency\n"
"  ll  - llvm\n"
"  bc  - llvm-bc\n"
"  s   - assembler\n"
@@ -628,6 +630,12 @@
 return new TextFileHandler(/*Comment=*/"//");
   if (FilesType == "ii")
 return new TextFileHandler(/*Comment=*/"//");
+  if (FilesType == "cui")
+return new TextFileHandler(/*Comment=*/"//");
+  // TODO: `.d` should be eventually removed once `-M` and its variants are
+  // handled properly in offload compilation.
+  if (FilesType == "d")
+return new TextFileHandler(/*Comment=*/"#");
   if (FilesType == "ll")
 return new TextFileHandler(/*Comment=*/";");
   if (FilesType == "bc")


Index: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -71,6 +71,8 @@
"Current supported types are:\n"
"  i   - cpp-output\n"
"  ii  - c++-cpp-output\n"
+   "  cui - cuda/hip-output\n"
+   "  d   - dependency\n"
"  ll  - llvm\n"
"  bc  - llvm-bc\n"
"  s   - assembler\n"
@@ -628,6 +630,12 @@
 return new TextFileHandler(/*Comment=*/"//");
   if (FilesType == "ii")
 return new TextFileHandler(/*Comment=*/"//");
+  if (FilesType == "cui")
+return new TextFileHandler(/*Comment=*/"//");
+  // TODO: `.d` should be eventually removed once `-M` and its variants are
+  // handled properly in offload compilation.
+  if (FilesType == "d")
+return new TextFileHandler(/*Comment=*/"#");
   if (FilesType == "ll")
 return new TextFileHandler(/*Comment=*/";");
   if (FilesType == "bc")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r374006 - Reland 'Add VFS support for sanitizers' blacklist'

[Nico Weber via cfe-commits]

FWIW reverting a patch for it breaking some internal system seems like poor
form to me. It's really hard to reland in that case. Please make a reduced
repro next time.

On Wed, Oct 9, 2019 at 5:38 AM Ilya Biryukov via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Reverted in r374151.
>
> On Wed, Oct 9, 2019 at 11:03 AM Ilya Biryukov 
> wrote:
>
>> Hi Jan,
>>
>> This patch seems to assume VFS will only re-map some paths, but still
>> point into the physical filesystem.
>> This may not be the case, e.g. in unit tests.
>>
>> This also manages to break some of our internal clang-tidy integrations
>> for obscure reasons.
>>
>> Can we instead just pass VFS instance to the SanitizerBlacklist and use
>> relative paths?
>>
>> We might also need to revert the patch to unbreak our release, sorry
>> about the inconvenience.
>>
>> On Tue, Oct 8, 2019 at 3:11 AM Jan Korous via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: jkorous
>>> Date: Mon Oct  7 18:13:17 2019
>>> New Revision: 374006
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=374006&view=rev
>>> Log:
>>> Reland 'Add VFS support for sanitizers' blacklist'
>>>
>>> The original patch broke the test for Windows.
>>> Trying to fix as per Reid's suggestions outlined here:
>>> https://reviews.llvm.org/rC371663
>>>
>>> Differential Revision: https://reviews.llvm.org/D67742
>>>
>>> Added:
>>> cfe/trunk/test/CodeGen/Inputs/sanitizer-blacklist-vfsoverlay.yaml
>>> Modified:
>>> cfe/trunk/lib/AST/ASTContext.cpp
>>> cfe/trunk/test/CodeGen/ubsan-blacklist.c
>>>
>>> Modified: cfe/trunk/lib/AST/ASTContext.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=374006&r1=374005&r2=374006&view=diff
>>>
>>> ==
>>> --- cfe/trunk/lib/AST/ASTContext.cpp (original)
>>> +++ cfe/trunk/lib/AST/ASTContext.cpp Mon Oct  7 18:13:17 2019
>>> @@ -72,6 +72,7 @@
>>>  #include "llvm/ADT/PointerUnion.h"
>>>  #include "llvm/ADT/STLExtras.h"
>>>  #include "llvm/ADT/SmallPtrSet.h"
>>> +#include "llvm/ADT/SmallString.h"
>>>  #include "llvm/ADT/SmallVector.h"
>>>  #include "llvm/ADT/StringExtras.h"
>>>  #include "llvm/ADT/StringRef.h"
>>> @@ -81,6 +82,7 @@
>>>  #include "llvm/Support/Compiler.h"
>>>  #include "llvm/Support/ErrorHandling.h"
>>>  #include "llvm/Support/MathExtras.h"
>>> +#include "llvm/Support/VirtualFileSystem.h"
>>>  #include "llvm/Support/raw_ostream.h"
>>>  #include 
>>>  #include 
>>> @@ -826,6 +828,18 @@ static bool isAddrSpaceMapManglingEnable
>>>llvm_unreachable("getAddressSpaceMapMangling() doesn't cover
>>> anything.");
>>>  }
>>>
>>> +static std::vector
>>> +getRealPaths(llvm::vfs::FileSystem &VFS, llvm::ArrayRef
>>> Paths) {
>>> +  std::vector Result;
>>> +  llvm::SmallString<128> Buffer;
>>> +  for (const auto &File : Paths) {
>>> +if (std::error_code EC = VFS.getRealPath(File, Buffer))
>>> +  llvm::report_fatal_error("can't open file '" + File + "': " +
>>> EC.message());
>>> +Result.push_back(Buffer.str());
>>> +  }
>>> +  return Result;
>>> +}
>>> +
>>>  ASTContext::ASTContext(LangOptions &LOpts, SourceManager &SM,
>>> IdentifierTable &idents, SelectorTable &sels,
>>> Builtin::Context &builtins)
>>> @@ -833,7 +847,10 @@ ASTContext::ASTContext(LangOptions &LOpt
>>>TemplateSpecializationTypes(this_()),
>>>DependentTemplateSpecializationTypes(this_()),
>>>SubstTemplateTemplateParmPacks(this_()), SourceMgr(SM),
>>> LangOpts(LOpts),
>>> -  SanitizerBL(new
>>> SanitizerBlacklist(LangOpts.SanitizerBlacklistFiles, SM)),
>>> +  SanitizerBL(new SanitizerBlacklist(
>>> +  getRealPaths(SM.getFileManager().getVirtualFileSystem(),
>>> +   LangOpts.SanitizerBlacklistFiles),
>>> +  SM)),
>>>XRayFilter(new
>>> XRayFunctionFilter(LangOpts.XRayAlwaysInstrumentFiles,
>>>
>>>  LangOpts.XRayNeverInstrumentFiles,
>>>  LangOpts.XRayAttrListFiles,
>>> SM)),
>>>
>>> Added: cfe/trunk/test/CodeGen/Inputs/sanitizer-blacklist-vfsoverlay.yaml
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/Inputs/sanitizer-blacklist-vfsoverlay.yaml?rev=374006&view=auto
>>>
>>> ==
>>> --- cfe/trunk/test/CodeGen/Inputs/sanitizer-blacklist-vfsoverlay.yaml
>>> (added)
>>> +++ cfe/trunk/test/CodeGen/Inputs/sanitizer-blacklist-vfsoverlay.yaml
>>> Mon Oct  7 18:13:17 2019
>>> @@ -0,0 +1,15 @@
>>> +{
>>> +  'version': 0,
>>> +  'roots': [
>>> +{ 'name': '@DIR@', 'type': 'directory',
>>> +  'contents': [
>>> +{ 'name': 'only-virtual-file.blacklist', 'type': 'file',
>>> +  'external-contents': '@REAL_FILE@'
>>> +},
>>> +{ 'name': 'invalid-virtual-file.blacklist', 'type': 'file',
>>> +  'external-contents': '@NONEXISTENT_FILE@'
>>> +}
>>> +  ]

[clang-tools-extra] r374168 - [clangd] Make sure ReplyCallbacks are destroyed before RequestCancelersMutex

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Wed Oct  9 06:59:31 2019
New Revision: 374168

URL: http://llvm.org/viewvc/llvm-project?rev=374168&view=rev
Log:
[clangd] Make sure ReplyCallbacks are destroyed before RequestCancelersMutex

Summary:
After rL374163, replycallbacks might have a cancellable context, which
will try to access RequestCancellers on destruction. See
http://45.33.8.238/mac/1245/step_7.txt for a sample failure.

Reviewers: sammccall

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits, 
thakis

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=374168&r1=374167&r2=374168&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Wed Oct  9 06:59:31 2019
@@ -371,16 +371,6 @@ private:
 
   llvm::StringMap> Notifications;
   llvm::StringMap> Calls;
-  // The maximum number of callbacks held in clangd.
-  //
-  // We bound the maximum size to the pending map to prevent memory leakage
-  // for cases where LSP clients don't reply for the request.
-  static constexpr int MaxReplayCallbacks = 100;
-  mutable std::mutex CallMutex;
-  int NextCallID = 0; /* GUARDED_BY(CallMutex) */
-  std::deque>>
-  ReplyCallbacks; /* GUARDED_BY(CallMutex) */
 
   // Method calls may be cancelled by ID, so keep track of their state.
   // This needs a mutex: handlers may finish on a different thread, and that's
@@ -432,6 +422,19 @@ private:
 }));
   }
 
+  // The maximum number of callbacks held in clangd.
+  //
+  // We bound the maximum size to the pending map to prevent memory leakage
+  // for cases where LSP clients don't reply for the request.
+  // This has to go after RequestCancellers and RequestCancellersMutex since it
+  // can contain a callback that has a cancelable context.
+  static constexpr int MaxReplayCallbacks = 100;
+  mutable std::mutex CallMutex;
+  int NextCallID = 0; /* GUARDED_BY(CallMutex) */
+  std::deque>>
+  ReplyCallbacks; /* GUARDED_BY(CallMutex) */
+
   ClangdLSPServer &Server;
 };
 constexpr int ClangdLSPServer::MessageHandler::MaxReplayCallbacks;


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

[PATCH] D68702: [clangd] Make sure ReplyCallbacks are destroyed before RequestCancelersMutex

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9a3a87d18975: [clangd] Make sure ReplyCallbacks are 
destroyed before RequestCancelersMutex (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68702

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp


Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -371,16 +371,6 @@
 
   llvm::StringMap> Notifications;
   llvm::StringMap> Calls;
-  // The maximum number of callbacks held in clangd.
-  //
-  // We bound the maximum size to the pending map to prevent memory leakage
-  // for cases where LSP clients don't reply for the request.
-  static constexpr int MaxReplayCallbacks = 100;
-  mutable std::mutex CallMutex;
-  int NextCallID = 0; /* GUARDED_BY(CallMutex) */
-  std::deque>>
-  ReplyCallbacks; /* GUARDED_BY(CallMutex) */
 
   // Method calls may be cancelled by ID, so keep track of their state.
   // This needs a mutex: handlers may finish on a different thread, and that's
@@ -432,6 +422,19 @@
 }));
   }
 
+  // The maximum number of callbacks held in clangd.
+  //
+  // We bound the maximum size to the pending map to prevent memory leakage
+  // for cases where LSP clients don't reply for the request.
+  // This has to go after RequestCancellers and RequestCancellersMutex since it
+  // can contain a callback that has a cancelable context.
+  static constexpr int MaxReplayCallbacks = 100;
+  mutable std::mutex CallMutex;
+  int NextCallID = 0; /* GUARDED_BY(CallMutex) */
+  std::deque>>
+  ReplyCallbacks; /* GUARDED_BY(CallMutex) */
+
   ClangdLSPServer &Server;
 };
 constexpr int ClangdLSPServer::MessageHandler::MaxReplayCallbacks;


Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -371,16 +371,6 @@
 
   llvm::StringMap> Notifications;
   llvm::StringMap> Calls;
-  // The maximum number of callbacks held in clangd.
-  //
-  // We bound the maximum size to the pending map to prevent memory leakage
-  // for cases where LSP clients don't reply for the request.
-  static constexpr int MaxReplayCallbacks = 100;
-  mutable std::mutex CallMutex;
-  int NextCallID = 0; /* GUARDED_BY(CallMutex) */
-  std::deque>>
-  ReplyCallbacks; /* GUARDED_BY(CallMutex) */
 
   // Method calls may be cancelled by ID, so keep track of their state.
   // This needs a mutex: handlers may finish on a different thread, and that's
@@ -432,6 +422,19 @@
 }));
   }
 
+  // The maximum number of callbacks held in clangd.
+  //
+  // We bound the maximum size to the pending map to prevent memory leakage
+  // for cases where LSP clients don't reply for the request.
+  // This has to go after RequestCancellers and RequestCancellersMutex since it
+  // can contain a callback that has a cancelable context.
+  static constexpr int MaxReplayCallbacks = 100;
+  mutable std::mutex CallMutex;
+  int NextCallID = 0; /* GUARDED_BY(CallMutex) */
+  std::deque>>
+  ReplyCallbacks; /* GUARDED_BY(CallMutex) */
+
   ClangdLSPServer &Server;
 };
 constexpr int ClangdLSPServer::MessageHandler::MaxReplayCallbacks;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68193: In openFileForRead don't cache erroneous entries if the error relates to them being directories. Add tests.

[Kousik Kumar via Phabricator via cfe-commits]

kousikk added a comment.

@arphaman I don't mind changing this if there are race conditions as you say, 
but isn't the assumption of the tool that the filesystem remains unchanged for 
a single run of the tool? If so, should we actually throw error conditions 
instead of crashing in those cases?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68193



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

[PATCH] D68702: [clangd] Make sure ReplyCallbacks are destroyed before RequestCancelersMutex

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 224045.
kadircet marked an inline comment as done.
kadircet added a comment.

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68702

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp


Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -371,16 +371,6 @@
 
   llvm::StringMap> Notifications;
   llvm::StringMap> Calls;
-  // The maximum number of callbacks held in clangd.
-  //
-  // We bound the maximum size to the pending map to prevent memory leakage
-  // for cases where LSP clients don't reply for the request.
-  static constexpr int MaxReplayCallbacks = 100;
-  mutable std::mutex CallMutex;
-  int NextCallID = 0; /* GUARDED_BY(CallMutex) */
-  std::deque>>
-  ReplyCallbacks; /* GUARDED_BY(CallMutex) */
 
   // Method calls may be cancelled by ID, so keep track of their state.
   // This needs a mutex: handlers may finish on a different thread, and that's
@@ -432,6 +422,19 @@
 }));
   }
 
+  // The maximum number of callbacks held in clangd.
+  //
+  // We bound the maximum size to the pending map to prevent memory leakage
+  // for cases where LSP clients don't reply for the request.
+  // This has to go after RequestCancellers and RequestCancellersMutex since it
+  // can contain a callback that has a cancelable context.
+  static constexpr int MaxReplayCallbacks = 100;
+  mutable std::mutex CallMutex;
+  int NextCallID = 0; /* GUARDED_BY(CallMutex) */
+  std::deque>>
+  ReplyCallbacks; /* GUARDED_BY(CallMutex) */
+
   ClangdLSPServer &Server;
 };
 constexpr int ClangdLSPServer::MessageHandler::MaxReplayCallbacks;


Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -371,16 +371,6 @@
 
   llvm::StringMap> Notifications;
   llvm::StringMap> Calls;
-  // The maximum number of callbacks held in clangd.
-  //
-  // We bound the maximum size to the pending map to prevent memory leakage
-  // for cases where LSP clients don't reply for the request.
-  static constexpr int MaxReplayCallbacks = 100;
-  mutable std::mutex CallMutex;
-  int NextCallID = 0; /* GUARDED_BY(CallMutex) */
-  std::deque>>
-  ReplyCallbacks; /* GUARDED_BY(CallMutex) */
 
   // Method calls may be cancelled by ID, so keep track of their state.
   // This needs a mutex: handlers may finish on a different thread, and that's
@@ -432,6 +422,19 @@
 }));
   }
 
+  // The maximum number of callbacks held in clangd.
+  //
+  // We bound the maximum size to the pending map to prevent memory leakage
+  // for cases where LSP clients don't reply for the request.
+  // This has to go after RequestCancellers and RequestCancellersMutex since it
+  // can contain a callback that has a cancelable context.
+  static constexpr int MaxReplayCallbacks = 100;
+  mutable std::mutex CallMutex;
+  int NextCallID = 0; /* GUARDED_BY(CallMutex) */
+  std::deque>>
+  ReplyCallbacks; /* GUARDED_BY(CallMutex) */
+
   ClangdLSPServer &Server;
 };
 constexpr int ClangdLSPServer::MessageHandler::MaxReplayCallbacks;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68166: [Clang][OpenMP Offload] Add new tool for wrapping offload device binaries

[Jon Chesterfield via Phabricator via cfe-commits]

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

The direction is good and I believe all the feedback from D64943 
 has already been incorporated. LGTM, thanks.


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

https://reviews.llvm.org/D68166



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

[PATCH] D68562: [clangd] Add RemoveUsingNamespace tweak.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Many thanks, this LG overall, just a few NITs and documentation requests.




Comment at: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp:31
+///   std::vector foo(std::map);
+class RemoveUsingNamespace : public Tweak {
+public:

Could you mention current limitation in the comment?
Something like
```
Currently limited to using namespace directives inside global namespace to 
simplify implementation.
```



Comment at: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp:85
+bool isTopLevelDecl(const SelectionTree::Node *Node) {
+  if (!Node->Parent)
+return false;

NIT: could be written as a single statement
```
return Node->Parent
&& Node->Parent->
```

up to you, though, both versions look fine.



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp:113
+return false;
+  // FIXME: Unavailable for namespaces containing using-namespace decl.
+  if (!TargetDirective->getNominatedNamespace()->using_directives().empty())

Could you provide the rationale why we do this too?
This information would be super-useful to folks fixing this later



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp:150
+  if (Loc.isMacroID()) {
+if (!SM.isMacroArgExpansion(Loc))
+  return; // FIXME: report a warning to the users.

Worth documenting why we do this here:
```
Avoid adding qualifiers before macro expansions, it's probably incorrect, e.g.
#define FOO 1 + matrix()
using namespace linalg; // provides matrix
auto x = FOO; 
 
Should not be changed to:
auto x = linalg::FOO;
```



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp:157
+return; // FIXME: report these to the user as warnings?
+  if (SM.isBeforeInTranslationUnit(Loc, FirstUsingDirectiveLoc))
+return;

NIT: could you add a comment on why we do this? Something like
```
Directive was not visible before this point.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68562



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

Re: r374006 - Reland 'Add VFS support for sanitizers' blacklist'

[Ilya Biryukov via cfe-commits]

Fair enough, sorry about that.

Nevertheless, I wanted to re-raise concerns about the approach taken in the
patch. It seems to assume invariants about VFS that don't necessarily hold
for filesystems outside those tested in upstream LLVM.
PTAL at the original patch - calling `getRealPath` on the FS and later
using real-file-system with the results assumes the passed VFS is an
overlay over the physical filesystem.
A simple unit-test with InMemoryFS would break right away, so the failing
case is pretty obvious.

There's an obvious alternative: just pass VFS to SanitizerBlacklist (in
fact, it already gets a source manager, which has a link to the VFS) and
use it for file accesses.
I would suggest doing another round of review on this, happy to take part
too.

On Wed, Oct 9, 2019 at 3:55 PM Nico Weber  wrote:

> FWIW reverting a patch for it breaking some internal system seems like
> poor form to me. It's really hard to reland in that case. Please make a
> reduced repro next time.
>
> On Wed, Oct 9, 2019 at 5:38 AM Ilya Biryukov via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Reverted in r374151.
>>
>> On Wed, Oct 9, 2019 at 11:03 AM Ilya Biryukov 
>> wrote:
>>
>>> Hi Jan,
>>>
>>> This patch seems to assume VFS will only re-map some paths, but still
>>> point into the physical filesystem.
>>> This may not be the case, e.g. in unit tests.
>>>
>>> This also manages to break some of our internal clang-tidy integrations
>>> for obscure reasons.
>>>
>>> Can we instead just pass VFS instance to the SanitizerBlacklist and use
>>> relative paths?
>>>
>>> We might also need to revert the patch to unbreak our release, sorry
>>> about the inconvenience.
>>>
>>> On Tue, Oct 8, 2019 at 3:11 AM Jan Korous via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
 Author: jkorous
 Date: Mon Oct  7 18:13:17 2019
 New Revision: 374006

 URL: http://llvm.org/viewvc/llvm-project?rev=374006&view=rev
 Log:
 Reland 'Add VFS support for sanitizers' blacklist'

 The original patch broke the test for Windows.
 Trying to fix as per Reid's suggestions outlined here:
 https://reviews.llvm.org/rC371663

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

 Added:
 cfe/trunk/test/CodeGen/Inputs/sanitizer-blacklist-vfsoverlay.yaml
 Modified:
 cfe/trunk/lib/AST/ASTContext.cpp
 cfe/trunk/test/CodeGen/ubsan-blacklist.c

 Modified: cfe/trunk/lib/AST/ASTContext.cpp
 URL:
 http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=374006&r1=374005&r2=374006&view=diff

 ==
 --- cfe/trunk/lib/AST/ASTContext.cpp (original)
 +++ cfe/trunk/lib/AST/ASTContext.cpp Mon Oct  7 18:13:17 2019
 @@ -72,6 +72,7 @@
  #include "llvm/ADT/PointerUnion.h"
  #include "llvm/ADT/STLExtras.h"
  #include "llvm/ADT/SmallPtrSet.h"
 +#include "llvm/ADT/SmallString.h"
  #include "llvm/ADT/SmallVector.h"
  #include "llvm/ADT/StringExtras.h"
  #include "llvm/ADT/StringRef.h"
 @@ -81,6 +82,7 @@
  #include "llvm/Support/Compiler.h"
  #include "llvm/Support/ErrorHandling.h"
  #include "llvm/Support/MathExtras.h"
 +#include "llvm/Support/VirtualFileSystem.h"
  #include "llvm/Support/raw_ostream.h"
  #include 
  #include 
 @@ -826,6 +828,18 @@ static bool isAddrSpaceMapManglingEnable
llvm_unreachable("getAddressSpaceMapMangling() doesn't cover
 anything.");
  }

 +static std::vector
 +getRealPaths(llvm::vfs::FileSystem &VFS, llvm::ArrayRef
 Paths) {
 +  std::vector Result;
 +  llvm::SmallString<128> Buffer;
 +  for (const auto &File : Paths) {
 +if (std::error_code EC = VFS.getRealPath(File, Buffer))
 +  llvm::report_fatal_error("can't open file '" + File + "': " +
 EC.message());
 +Result.push_back(Buffer.str());
 +  }
 +  return Result;
 +}
 +
  ASTContext::ASTContext(LangOptions &LOpts, SourceManager &SM,
 IdentifierTable &idents, SelectorTable &sels,
 Builtin::Context &builtins)
 @@ -833,7 +847,10 @@ ASTContext::ASTContext(LangOptions &LOpt
TemplateSpecializationTypes(this_()),
DependentTemplateSpecializationTypes(this_()),
SubstTemplateTemplateParmPacks(this_()), SourceMgr(SM),
 LangOpts(LOpts),
 -  SanitizerBL(new
 SanitizerBlacklist(LangOpts.SanitizerBlacklistFiles, SM)),
 +  SanitizerBL(new SanitizerBlacklist(
 +  getRealPaths(SM.getFileManager().getVirtualFileSystem(),
 +   LangOpts.SanitizerBlacklistFiles),
 +  SM)),
XRayFilter(new
 XRayFunctionFilter(LangOpts.XRayAlwaysInstrumentFiles,

  LangOpts.XRayNeverInstrumentFiles,
  LangOpts.XRayAttrListFiles,
 

-isysem path

[Monalisa Rout via cfe-commits]

Hello,

I have created one visual studio application by using LibTooling library.
I am dumping the AST and all LLVM Types. I want to pass the isystem path to
the
clangTool object.

I have declared the isystem path in the Command Arguments
(Project->Properties->Debugging->Command Arguments)
input.c Output.json -isystem xxx

static llvm::cl::list ISystemPaths(
"isystem", llvm::cl::ZeroOrMore, llvm::cl::value_desc("filename"),
llvm::cl::desc("This is isystem"));

I have read that command argument as well, but I have no idea how to use it
further.

Have a look at my source code if that would help you to understand the
problem clearly.

[ clang -Xclang -ast-dump input.c -isystem xxx ] - I dont want to use like
this.

Regards,
Mona

#include
using namespace std;
#include 
#include 

#include "clang/AST/AST.h"
#include "clang/AST/ASTConsumer.h"
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/Frontend/ASTConsumers.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/FrontendActions.h"
#include "clang/Rewrite/Core/Rewriter.h"
#include "clang/Tooling/CommonOptionsParser.h"
#include "clang/Tooling/Tooling.h"
#include "llvm/Support/raw_ostream.h"
#include 
#include "llvm/Support/JSON.h"
#include "clang/AST/JSONNodeDumper.h"
#include "clang/Basic/DiagnosticIDs.h"
using namespace clang;
using namespace clang::driver;
using namespace clang::tooling;

static llvm::cl::OptionCategory ToolingSampleCategory("Tooling Sample");

static llvm::cl::list ISystemPaths(
"isystem", llvm::cl::ZeroOrMore, llvm::cl::value_desc("filename"),
llvm::cl::desc("This is isystem"));


StringRef 
stringref1("D:\\Data\\Rout\\svn\\ASTJSONProject\\ASTJSONProject\\defaultOutput.json");
llvm::raw_fd_ostream Sdefault(stringref1, std::error_code());
std::string outputPath;
// By implementing RecursiveASTVisitor, we can specify which AST nodes
// we're interested in by overriding relevant methods.
class MyASTVisitor : public RecursiveASTVisitor {
public:
MyASTVisitor(Rewriter &R) : TheRewriter(R) {}

bool VisitStmt(Stmt *s) {

return true;
}

// how to make out function def, fuction call and function declaration.
bool VisitFunctionDecl(FunctionDecl *f) {
// Only function definitions (with bodies), not declarations.
if (f->hasBody()) {
Stmt *FuncBody = f->getBody();
// Type name as string
QualType QT = f->getReturnType();
std::string TypeStr = QT.getAsString();

// Function name
DeclarationName DeclName = f->getNameInfo().getName();
std::string FuncName = DeclName.getAsString();
}

return true;
}
bool VisitDecl(Decl *D) 
{ 
if (isa(D))
return true;
}

bool VisitExpr(const Expr *E)
{


return true;
}
bool VisitRecordDecl(RecordDecl *Declaration) {
// For debugging, dumping the AST nodes will show which nodes 
are already
// being visited.
//Declaration->dump(llvm::errs(), false, clang::ADOF_JSON);

// The return value indicates whether we want the visitation to 
proceed.
// Return false to stop the traversal of the AST.
return true;
}

private:
Rewriter & TheRewriter;
//private Block block; // available in Gecos library
};


// Implementation of the ASTConsumer interface for reading an AST produced
// by the Clang parser.


class MyASTConsumer : public ASTConsumer {
public:
llvm::raw_fd_ostream S = { StringRef(outputPath) , std::error_code() };
MyASTConsumer(Rewriter &R) : Visitor(R) {
/*StringRef stringref(outputPath);
llvm::raw_fd_ostream S(stringref, std::error_code());*/
S << "{";
S<< "\n";
std::string str("\"root\"");
S << str;
S << ":";
S<< "\n";
S << "[";
S<< "\n";
}

// Override the method that gets called for each parsed top-level
// declaration.
bool HandleTopLevelDecl(DeclGroupRef DR) override { 

for (DeclGroupRef::iterator b = DR.begin(), e = DR.end(); b != 
e; ++b) {
// Traverse the declaration using our AST visitor.
RecordDecl *recordDecl = dyn_cast(*b);
TypedefDecl* typedefdecl = dyn_cast(*b);
if (recordDecl)
{
std::vector recordVec;
//const struct
QualType reco

r374153 - [DebugInfo] Enable call site debug info for ARM and AArch64

[Nikola Prica via cfe-commits]

Author: nikolaprica
Date: Wed Oct  9 03:14:15 2019
New Revision: 374153

URL: http://llvm.org/viewvc/llvm-project?rev=374153&view=rev
Log:
[DebugInfo] Enable call site debug info for ARM and AArch64

ARM and AArch64 SelectionDAG support for tacking parameter forwarding
register is implemented so we can allow clang invocations for those two
targets.
Beside that restrict debug entry value support to be emitted for
LimitedDebugInfo info and FullDebugInfo. Other types of debug info do
not have functions nor variables debug info.

Reviewers: aprantl, probinson, dstenb, vsk

Reviewed By: vsk

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

Modified:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/CodeGen/debug-info-param-modification.c

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=374153&r1=374152&r2=374153&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Wed Oct  9 03:14:15 2019
@@ -3706,8 +3706,7 @@ void CGDebugInfo::EmitFuncDeclForCallSit
   const FunctionDecl *CalleeDecl) {
   auto &CGOpts = CGM.getCodeGenOpts();
   if (!CGOpts.EnableDebugEntryValues || !CGM.getLangOpts().Optimize ||
-  !CallOrInvoke ||
-  CGM.getCodeGenOpts().getDebugInfo() < codegenoptions::LimitedDebugInfo)
+  !CallOrInvoke)
 return;
 
   auto *Func = CallOrInvoke->getCalledFunction();

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=374153&r1=374152&r2=374153&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Wed Oct  9 03:14:15 2019
@@ -777,10 +777,14 @@ static bool ParseCodeGenArgs(CodeGenOpti
   Opts.DisableLLVMPasses = Args.hasArg(OPT_disable_llvm_passes);
   Opts.DisableLifetimeMarkers = Args.hasArg(OPT_disable_lifetimemarkers);
 
+  const llvm::Triple::ArchType DebugEntryValueArchs[] = {
+  llvm::Triple::x86, llvm::Triple::x86_64, llvm::Triple::aarch64,
+  llvm::Triple::arm, llvm::Triple::armeb};
+
   llvm::Triple T(TargetOpts.Triple);
-  llvm::Triple::ArchType Arch = T.getArch();
   if (Opts.OptimizationLevel > 0 &&
-  (Arch == llvm::Triple::x86 || Arch == llvm::Triple::x86_64))
+  Opts.getDebugInfo() >= codegenoptions::LimitedDebugInfo &&
+  llvm::is_contained(DebugEntryValueArchs, T.getArch()))
 Opts.EnableDebugEntryValues = Args.hasArg(OPT_femit_debug_entry_values);
 
   Opts.DisableO0ImplyOptNone = Args.hasArg(OPT_disable_O0_optnone);

Modified: cfe/trunk/test/CodeGen/debug-info-param-modification.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/debug-info-param-modification.c?rev=374153&r1=374152&r2=374153&view=diff
==
--- cfe/trunk/test/CodeGen/debug-info-param-modification.c (original)
+++ cfe/trunk/test/CodeGen/debug-info-param-modification.c Wed Oct  9 03:14:15 
2019
@@ -1,4 +1,8 @@
 // RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -Xclang 
-disable-llvm-passes -S -target x86_64-none-linux-gnu -emit-llvm %s -o - | 
FileCheck %s -check-prefix=CHECK-ENTRY-VAL-OPT
+// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -Xclang 
-disable-llvm-passes -S -target arm-none-linux-gnu -emit-llvm %s -o - | 
FileCheck %s -check-prefix=CHECK-ENTRY-VAL-OPT
+// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -Xclang 
-disable-llvm-passes -S -target aarch64-none-linux-gnu -emit-llvm %s -o - | 
FileCheck %s -check-prefix=CHECK-ENTRY-VAL-OPT
+// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -Xclang 
-disable-llvm-passes -S -target armeb-none-linux-gnu -emit-llvm %s -o - | 
FileCheck %s -check-prefix=CHECK-ENTRY-VAL-OPT
+
 // CHECK-ENTRY-VAL-OPT: !DILocalVariable(name: "a", arg: 1, scope: {{.*}}, 
file: {{.*}}, line: {{.*}}, type: {{.*}})
 // CHECK-ENTRY-VAL-OPT: !DILocalVariable(name: "b", arg: 2, scope: {{.*}}, 
file: {{.*}}, line: {{.*}}, type: {{.*}}, flags: DIFlagArgumentNotModified)
 //


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

r374172 - [clang-format] Update noexcept reference qualifiers detection

[Krasimir Georgiev via cfe-commits]

Author: krasimir
Date: Wed Oct  9 07:46:08 2019
New Revision: 374172

URL: http://llvm.org/viewvc/llvm-project?rev=374172&view=rev
Log:
[clang-format] Update noexcept reference qualifiers detection

Summary:
r373165 fixed an issue where a templated noexcept member function with a
reference qualifier would be indented more than expected:
```
// Formatting produced with LLVM style with AlwaysBreakTemplateDeclarations: Yes

// before r373165:
struct f {
  template 
  void bar() && noexcept {}
};

// after:
struct f {
  template 
  void bar() && noexcept {}
};

```
The way this is done is that in the AnnotatingParser in
`lib/FormatTokenAnnotator.cpp` the determination of the usage of a `&` or `&&`
(the line in determineTokenType

```
Current.Type = determineStarAmpUsage(...
```
is not performed in some cases anymore, combining with a few additional related
checks afterwards. The net effect of these checks results in the `&` or `&&`
token to start being classified as `TT_Unknown` in cases where before `r373165`
it would be classified as `TT_UnaryOperator` or `TT_PointerOrReference` by
`determineStarAmpUsage`.

This inadvertently caused 2 classes of regressions I'm aware of:

- The address-of `&` after a function assignment would be classified as
  `TT_Unknown`, causing spaces to surround it, disregarding style options:
```
// before r373165:
void (*fun_ptr)(void) = &fun;

// after:
void (*fun_ptr)(void) = & fun;
```

- In cases where there is a function declaration list -- looking macro between
  a template line and the start of the function declaration, an `&` as part of
  the return type would be classified as `TT_Unknown`, causing spaces to
  surround it:
```
// before r373165:
template 
DEPRECATED("lala")
Type& foo();

// after:
template 
DEPRECATED("lala")
Type & foo();
```

In these cases the problems are rooted in the skipping of the classification of
a `&` (and similarly `&&`) by determineStarAmpUsage which effects the formatting
decisions later in the pipeline.

I've looked into the goal of r373165 and noticed that replacing `noexcept` with
`const` in the given example produces no extra indentation with the old code:
```
// before r373165:
struct f {
  template 
  int foo() & const {}
};

struct f {
  template 
  int foo() & noexcept {}
};
```

I investigated how clang-format annotated these two examples differently to
determine the places where the processing of both diverges in the pipeline.
There were two places where the processing diverges, causing the extra indent in
the `noexcept` case:
1. The `const` is annotated as a `TT_TrailingAnnotation`, whereas `noexcept`
   is annotated as `TT_Unknown`. I've updated the `determineTokenType` function
   to account for this by adding a missing `tok:kw_noexcept` to the clause that
   marks a token as `TT_TrailingAnnotation`.
2. The `&` in the second example is wrongly identified as `TT_BinaryOperator`
   in `determineStarAmpUsage`. This is the reason for the extra indentation --
   clang-format gets confused and thinks this is an expression.
   I've updated `determineStarAmpUsage` to check for `tok:kw_noexcept`.

With these two updates in place, the additional parsing introduced by r373165
becomes unnecessary and all added tests pass (with updates, as now clang-format
respects the style configuration for spaces around the `&` in the test
examples).
I've removed these additions and added regression tests for the cases above.

Reviewers: AndWass, MyDeveloperDay

Reviewed By: MyDeveloperDay

Subscribers: cfe-commits

Tags: #clang, #clang-format

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

Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=374172&r1=374171&r2=374172&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Wed Oct  9 07:46:08 2019
@@ -15,6 +15,7 @@
 #include "TokenAnnotator.h"
 #include "FormatToken.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TokenKinds.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/Support/Debug.h"
 
@@ -65,7 +66,7 @@ public:
   AnnotatingParser(const FormatStyle &Style, AnnotatedLine &Line,
const AdditionalKeywords &Keywords)
   : Style(Style), Line(Line), CurrentToken(Line.First), AutoFound(false),
-TrailingReturnFound(false), Keywords(Keywords) {
+Keywords(Keywords) {
 Contexts.push_back(Context(tok::unknown, 1, /*IsExpression=*/false));
 resetTokenMetadata(CurrentToken);
   }
@@ -1397,11 +1398,7 @@ private:
!Current.Previous->is(tok::kw_operator)) {
   // not auto operator->() -> xxx;
   Current.Type = TT_TrailingReturnArrow;
-  TrailingReturnFound = true;
-} else if (Current.is(tok::st

[PATCH] D68695: [clang-format] Update noexcept reference qualifiers detection

[Krasimir Georgiev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
krasimir marked an inline comment as done.
Closed by commit rGae1b7859cbd6: [clang-format] Update noexcept reference 
qualifiers detection (authored by krasimir).

Changed prior to commit:
  https://reviews.llvm.org/D68695?vs=224022&id=224058#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68695

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
@@ -7037,31 +7037,31 @@
 
   verifyFormat("struct f {\n"
"  template \n"
-   "  int &foo(const std::string &str) & noexcept {}\n"
+   "  int &foo(const std::string &str) &noexcept {}\n"
"};",
BreakTemplate);
 
   verifyFormat("struct f {\n"
"  template \n"
-   "  int &foo(const std::string &str) && noexcept {}\n"
+   "  int &foo(const std::string &str) &&noexcept {}\n"
"};",
BreakTemplate);
 
   verifyFormat("struct f {\n"
"  template \n"
-   "  int &foo(const std::string &str) const & noexcept {}\n"
+   "  int &foo(const std::string &str) const &noexcept {}\n"
"};",
BreakTemplate);
 
   verifyFormat("struct f {\n"
"  template \n"
-   "  int &foo(const std::string &str) const & noexcept {}\n"
+   "  int &foo(const std::string &str) const &noexcept {}\n"
"};",
BreakTemplate);
 
   verifyFormat("struct f {\n"
"  template \n"
-   "  auto foo(const std::string &str) && noexcept -> int & {}\n"
+   "  auto foo(const std::string &str) &&noexcept -> int & {}\n"
"};",
BreakTemplate);
 
@@ -7084,13 +7084,13 @@
 
   verifyFormat("struct f {\n"
"  template \n"
-   "  int& foo(const std::string& str) const & noexcept {}\n"
+   "  int& foo(const std::string& str) const& noexcept {}\n"
"};",
AlignLeftBreakTemplate);
 
   verifyFormat("struct f {\n"
"  template \n"
-   "  int& foo(const std::string& str) const & noexcept {}\n"
+   "  int& foo(const std::string& str) const&& noexcept {}\n"
"};",
AlignLeftBreakTemplate);
 
@@ -7099,6 +7099,24 @@
"  auto foo(const std::string& str) && noexcept -> int& {}\n"
"};",
AlignLeftBreakTemplate);
+
+  // The `&` in `Type&` should not be confused with a trailing `&` of
+  // DEPRECATED(reason) member function.
+  verifyFormat("struct f {\n"
+   "  template \n"
+   "  DEPRECATED(reason)\n"
+   "  Type &foo(arguments) {}\n"
+   "};",
+   BreakTemplate);
+
+  verifyFormat("struct f {\n"
+   "  template \n"
+   "  DEPRECATED(reason)\n"
+   "  Type& foo(arguments) {}\n"
+   "};",
+   AlignLeftBreakTemplate);
+
+  verifyFormat("void (*foopt)(int) = &func;");
 }
 
 TEST_F(FormatTest, UnderstandsNewAndDelete) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -15,6 +15,7 @@
 #include "TokenAnnotator.h"
 #include "FormatToken.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TokenKinds.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/Support/Debug.h"
 
@@ -65,7 +66,7 @@
   AnnotatingParser(const FormatStyle &Style, AnnotatedLine &Line,
const AdditionalKeywords &Keywords)
   : Style(Style), Line(Line), CurrentToken(Line.First), AutoFound(false),
-TrailingReturnFound(false), Keywords(Keywords) {
+Keywords(Keywords) {
 Contexts.push_back(Context(tok::unknown, 1, /*IsExpression=*/false));
 resetTokenMetadata(CurrentToken);
   }
@@ -1397,11 +1398,7 @@
!Current.Previous->is(tok::kw_operator)) {
   // not auto operator->() -> xxx;
   Current.Type = TT_TrailingReturnArrow;
-  TrailingReturnFound = true;
-} else if (Current.is(tok::star) ||
-   (Current.isOneOf(tok::amp, tok::ampamp) &&
-(Current.NestingLevel != 0 || !Line.MightBeFunctionDecl ||
- TrailingReturnFound))) {
+} else if (Current.isOneOf(tok::star, tok::amp, tok::ampamp)) {
   Current.Type = determineStarAmpUsage(Current,
Contexts.back().CanBeExpression &&
Contexts.back(

r374174 - [NFC] Test commit.

[Mitchell Balan via cfe-commits]

Author: mitchell
Date: Wed Oct  9 08:11:34 2019
New Revision: 374174

URL: http://llvm.org/viewvc/llvm-project?rev=374174&view=rev
Log:
[NFC] Test commit.
Testing llvm commit access only.

Modified:
cfe/trunk/docs/ClangFormatStyleOptions.rst

Modified: cfe/trunk/docs/ClangFormatStyleOptions.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangFormatStyleOptions.rst?rev=374174&r1=374173&r2=374174&view=diff
==
--- cfe/trunk/docs/ClangFormatStyleOptions.rst (original)
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst Wed Oct  9 08:11:34 2019
@@ -2507,3 +2507,5 @@ The result is:
   r();
   }
   }
+
+  
\ No newline at end of file


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

r374175 - [NFC] Reverting changes from test commit.

[Mitchell Balan via cfe-commits]

Author: mitchell
Date: Wed Oct  9 08:12:22 2019
New Revision: 374175

URL: http://llvm.org/viewvc/llvm-project?rev=374175&view=rev
Log:
[NFC] Reverting changes from test commit.
llvm commit access test succeeded.

Modified:
cfe/trunk/docs/ClangFormatStyleOptions.rst

Modified: cfe/trunk/docs/ClangFormatStyleOptions.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangFormatStyleOptions.rst?rev=374175&r1=374174&r2=374175&view=diff
==
--- cfe/trunk/docs/ClangFormatStyleOptions.rst (original)
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst Wed Oct  9 08:12:22 2019
@@ -2507,5 +2507,3 @@ The result is:
   r();
   }
   }
-
-  
\ No newline at end of file


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

[PATCH] D68590: [clangd] Improve hover support for Objective-C

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Thanks! This generally looks good, just need to find the right home for some of 
the logic.




Comment at: clangd/XRefs.cpp:455
 
+static std::string getNameForObjCMethod(const ObjCMethodDecl *Method) {
+  std::string Name;

XRefs.cpp isn't really the right place for this.
If there isn't somewhere appropriate in clangAST, then making this a case of 
printName in AST.h is probably best.

It should have unit tests (rather than testing it all through hover)



Comment at: clangd/XRefs.cpp:532
 
+  // Don't consider Type/Function/ObjC decls to be a namespace. Instead, check
+  // where they themselves are declared by recursing.

Maybe "skip over Type/Function/ObjC methods, these are part of the local scope 
instead".

(Not sure about "objc decl" as it seems to cover a lot of other things too)



Comment at: clangd/unittests/XRefsTests.cpp:1029
 
+TEST(Hover, ObjectiveC) {
+  struct {

combine test cases with above.
Do we need so many? I think 2 plus some unit-tests of the "pretty-print a decl" 
code should be enough



Comment at: clangd/unittests/XRefsTests.cpp:1177
+  };
+  for (const auto &Case : Cases) {
+SCOPED_TRACE(Case.Code);

you've got a loop here but only one test case. Combine with the other loop or 
just assert directly?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68590



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

[PATCH] D67536: [WIP] [clangd] Add support for an inactive regions notification

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

In D67536#1701038 , @ilya-biryukov 
wrote:

> In D67536#1700872 , @nridge wrote:
>
> > > It also lets them consistently highlight part of the line (e.g. dead 
> > > expressions or statements can be marked in gray even if they are on the 
> > > same line).
> >
> > Highlighting part of a line is not applicable to inactive preprocessor 
> > branches in C++.
>
>
> Note that the opposite is true - inactive preprocessor branches can be 
> expressed as range-based highlightings.


Except that if we want to allow background styling, the client would need to 
special-case the highlighting scope to know to apply the background style to 
the entire line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67536



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

[PATCH] D68707: [clang-format] throws an incorrect assertion in consumeToken() formatting the MSVC stl

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: mitchell-stellar, STL_MSFT, klimek, krasimir.
MyDeveloperDay added projects: clang-format, clang-tools-extra.
Herald added a project: clang.
MyDeveloperDay edited the summary of this revision.

An incorrect assertion is thrown when clang-formatting MSVC's STL library

  Assertion failed: !Line.startsWith(tok::hash), file 
C:/llvm/llvm-project/clang/lib/Format/TokenAnnotator.cpp, line 847
  Stack dump:
  0.  Program arguments: C:\llvm\build\bin\clang-format.exe -i -n 
./stl/inc/xkeycheck.h



  #if defined(while)
  #define while EMIT WARNING C4005
  #error The C++ Standard Library forbids macroizing the keyword "while". \
  Enable warning C4005 to find the forbidden define.
  #endif // while


Repository:
  rC Clang

https://reviews.llvm.org/D68707

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
@@ -14679,6 +14679,12 @@
   */
 }
 
-} // end namespace
-} // end namespace format
-} // end namespace clang
+TEST_F(FormatTest, STLWhileNotDefineChed) {
+  verifyFormat("#if defined(while)\n"
+   "#define while EMIT WARNING C4005\n"
+   "#endif // while");
+}
+
+} // namespace
+} // namespace format
+} // namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -844,14 +844,15 @@
   break;
 case tok::kw_if:
 case tok::kw_while:
-  assert(!Line.startsWith(tok::hash));
-  if (Tok->is(tok::kw_if) && CurrentToken &&
-  CurrentToken->isOneOf(tok::kw_constexpr, tok::identifier))
-next();
-  if (CurrentToken && CurrentToken->is(tok::l_paren)) {
-next();
-if (!parseParens(/*LookForDecls=*/true))
-  return false;
+  if (!Line.startsWith(tok::hash)) {
+if (Tok->is(tok::kw_if) && CurrentToken &&
+CurrentToken->isOneOf(tok::kw_constexpr, tok::identifier))
+  next();
+if (CurrentToken && CurrentToken->is(tok::l_paren)) {
+  next();
+  if (!parseParens(/*LookForDecls=*/true))
+return false;
+}
   }
   break;
 case tok::kw_for:


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14679,6 +14679,12 @@
   */
 }
 
-} // end namespace
-} // end namespace format
-} // end namespace clang
+TEST_F(FormatTest, STLWhileNotDefineChed) {
+  verifyFormat("#if defined(while)\n"
+   "#define while EMIT WARNING C4005\n"
+   "#endif // while");
+}
+
+} // namespace
+} // namespace format
+} // namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -844,14 +844,15 @@
   break;
 case tok::kw_if:
 case tok::kw_while:
-  assert(!Line.startsWith(tok::hash));
-  if (Tok->is(tok::kw_if) && CurrentToken &&
-  CurrentToken->isOneOf(tok::kw_constexpr, tok::identifier))
-next();
-  if (CurrentToken && CurrentToken->is(tok::l_paren)) {
-next();
-if (!parseParens(/*LookForDecls=*/true))
-  return false;
+  if (!Line.startsWith(tok::hash)) {
+if (Tok->is(tok::kw_if) && CurrentToken &&
+CurrentToken->isOneOf(tok::kw_constexpr, tok::identifier))
+  next();
+if (CurrentToken && CurrentToken->is(tok::l_paren)) {
+  next();
+  if (!parseParens(/*LookForDecls=*/true))
+return false;
+}
   }
   break;
 case tok::kw_for:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67160: [clang, ARM] Default to -fno-lax-vector-conversions in ARM v8.1-M.

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham added a comment.

Yes, the commit message in rL371817  
mentions that the long-term aim is to change the default to `none` for 
everyone. If that change happens before I manage to land this patch, then this 
patch certainly will become unnecessary.

But D68683  doesn't make that change: it only 
prepares `arm_neon.h` not to fall over in future when the change does happen. 
And MVE needs strict vector type checking more urgently than anyone else 
(because for us, it's not just a nice-to-have error-checking improvement, but 
critical to the polymorphic ACLE intrinsics working properly in the first 
place).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67160



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

[PATCH] D68707: [clang-format] throws an incorrect assertion in consumeToken() formatting the MSVC stl

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:847
 case tok::kw_while:
-  assert(!Line.startsWith(tok::hash));
-  if (Tok->is(tok::kw_if) && CurrentToken &&
-  CurrentToken->isOneOf(tok::kw_constexpr, tok::identifier))
-next();
-  if (CurrentToken && CurrentToken->is(tok::l_paren)) {
-next();
-if (!parseParens(/*LookForDecls=*/true))
-  return false;
+  if (!Line.startsWith(tok::hash)) {
+if (Tok->is(tok::kw_if) && CurrentToken &&

It's not clear to me whether or not the token should be consumed. The previous 
assertion leads me to think no, and in that case, I think this should be
```
if (Line.startsWith(tok::hash))
return false;
```
A comment on this would also be helpful.


Repository:
  rC Clang

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

https://reviews.llvm.org/D68707



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

[PATCH] D68562: [clangd] Add RemoveUsingNamespace tweak.

[UTKARSH SAXENA via Phabricator via cfe-commits]

usaxena95 updated this revision to Diff 224067.
usaxena95 marked 5 inline comments as done.
usaxena95 added a comment.

Added documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68562

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/AST.h
  clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
  clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -69,7 +69,8 @@
   EXPECT_EQ(apply("^if (true) {return 100;} else {continue;}"),
 "if (true) {continue;} else {return 100;}");
   EXPECT_EQ(apply("^if () {return 100;} else {continue;}"),
-"if () {continue;} else {return 100;}") << "broken condition";
+"if () {continue;} else {return 100;}")
+  << "broken condition";
   EXPECT_AVAILABLE("^i^f^^(^t^r^u^e^) { return 100; } ^e^l^s^e^ { continue; }");
   EXPECT_UNAVAILABLE("if (true) {^return ^100;^ } else { ^continue^;^ }");
   // Available in subexpressions of the condition;
@@ -100,7 +101,7 @@
   EXPECT_UNAVAILABLE(R"cpp(R"(multi )" ^"token " u8"str\ning")cpp"); // nonascii
   EXPECT_UNAVAILABLE(R"cpp(^R^"^(^multi )" "token " "str\ning")cpp"); // raw
   EXPECT_UNAVAILABLE(R"cpp(^"token\n" __FILE__)cpp"); // chunk is macro
-  EXPECT_UNAVAILABLE(R"cpp(^"a\r\n";)cpp"); // forbidden escape char
+  EXPECT_UNAVAILABLE(R"cpp(^"a\r\n";)cpp");   // forbidden escape char
 
   const char *Input = R"cpp(R"(multi
 token)" "\nst^ring\n" "literal")cpp";
@@ -286,11 +287,11 @@
  void f(int a) {
int y = PLUS([[1+a]]);
  })cpp",
-  /*FIXME: It should be extracted like this.
-   R"cpp(#define PLUS(x) x++
- void f(int a) {
-   auto dummy = 1+a; int y = PLUS(dummy);
- })cpp"},*/
+   /*FIXME: It should be extracted like this.
+R"cpp(#define PLUS(x) x++
+  void f(int a) {
+auto dummy = 1+a; int y = PLUS(dummy);
+  })cpp"},*/
R"cpp(#define PLUS(x) x++
  void f(int a) {
auto dummy = PLUS(1+a); int y = dummy;
@@ -301,13 +302,13 @@
if(1)
 LOOP(5 + [[3]])
  })cpp",
-  /*FIXME: It should be extracted like this. SelectionTree needs to be
-* fixed for macros.
-   R"cpp(#define LOOP(x) while (1) {a = x;}
-   void f(int a) {
- auto dummy = 3; if(1)
-  LOOP(5 + dummy)
-   })cpp"},*/
+   /*FIXME: It should be extracted like this. SelectionTree needs to be
+ * fixed for macros.
+R"cpp(#define LOOP(x) while (1) {a = x;}
+void f(int a) {
+  auto dummy = 3; if(1)
+   LOOP(5 + dummy)
+})cpp"},*/
R"cpp(#define LOOP(x) while (1) {a = x;}
  void f(int a) {
auto dummy = LOOP(5 + 3); if(1)
@@ -403,8 +404,8 @@
  void f() {
auto dummy = S(2) + S(3) + S(4); S x = S(1) + dummy + S(5);
  })cpp"},
-   // Don't try to analyze across macro boundaries
-   // FIXME: it'd be nice to do this someday (in a safe way)
+  // Don't try to analyze across macro boundaries
+  // FIXME: it'd be nice to do this someday (in a safe way)
   {R"cpp(#define ECHO(X) X
  void f() {
int x = 1 + [[ECHO(2 + 3) + 4]] + 5;
@@ -521,7 +522,7 @@
   StartsWith("fail: Could not expand type of lambda expression"));
   // inline namespaces
   EXPECT_EQ(apply("au^to x = inl_ns::Visible();"),
-  "Visible x = inl_ns::Visible();");
+"Visible x = inl_ns::Visible();");
   // local class
   EXPECT_EQ(apply("namespace x { void y() { struct S{}; ^auto z = S(); } }"),
 "namespace x { void y() { struct S{}; S z = S(); } }");
@@ -656,6 +657,208 @@
   EXPECT_THAT(apply(" for(;;) { [[while(1) break; break;]] }"),
   StartsWith("fail"));
 }
+
+TWEAK_TEST(RemoveUsingNamespace);
+TEST_F(RemoveUsingNamespaceTest, All) {
+  std::pair Cases[] = {
+  {// Remove all occurrences of ns. Qualify only unqualified.
+   R"cpp(
+  namespace ns1 { struct vector {}; }
+  namespace ns2 { struct map {}; }
+  using namespace n^s1;
+  using namespace ns2;
+  using namespace ns1;
+  int main() {
+ns1::vector v1;
+vector v2;
+map m1;
+  }
+)cpp",
+   R"cpp(
+  namespace ns1 { struct vector {}; }
+  namespace

[PATCH] D68710: Remove time-trace message as it inconsistent style

[Russell Gallop via Phabricator via cfe-commits]

russell.gallop created this revision.
russell.gallop added reviewers: anton-afanasyev, sylvestre.ledru.
Herald added a project: clang.

https://reviews.llvm.org/D68260 removed some of the message that -ftime-trace 
prints out. Running large builds still prints out a lot of "Time trace 
json-file dumped to " messages which are not the normal style for clang and are 
not necessary for locating the trace file.

There are other options which create files but don't tell you where they've 
written them. For example, this command generates a time trace, an yaml 
optimization record and a dependency file but only tells you about the time 
trace file:

$ clang -c foo.cpp -ftime-trace -fsave-optimisation-record -MD
Time trace json-file dumped to foo.json
$ ls
foo.cpp foo.d foo.json foo.o foo.opt.yaml

Supplemented the command line reference to help users locate this file and 
updated Options.td which is used to generate ClangCommandLineReference and 
clang --help.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68710

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td
  clang/tools/driver/cc1_main.cpp


Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -269,8 +269,6 @@
   // FIXME(ibiryukov): make profilerOutput flush in destructor instead.
   profilerOutput->flush();
   llvm::timeTraceProfilerCleanup();
-
-  llvm::errs() << "Time trace json-file dumped to " << Path.str() << "\n";
 }
   }
 
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1793,7 +1793,10 @@
 def fthreadsafe_statics : Flag<["-"], "fthreadsafe-statics">, Group;
 def ftime_report : Flag<["-"], "ftime-report">, Group, 
Flags<[CC1Option]>;
 def ftime_trace : Flag<["-"], "ftime-trace">, Group,
-  HelpText<"Turn on time profiler">, Flags<[CC1Option, CoreOption]>;
+  HelpText<"Turn on time profiler. Generates JSON file based on output 
filename. "
+   "Results can be analyzed with chrome://tracing or `Speedscope App "
+   "`_ for flamegraph visualization." >,
+  Flags<[CC1Option, CoreOption]>;
 def ftime_trace_granularity_EQ : Joined<["-"], "ftime-trace-granularity=">, 
Group,
   HelpText<"Minimum time granularity (in microseconds) traced by time 
profiler">,
   Flags<[CC1Option, CoreOption]>;
Index: clang/docs/ClangCommandLineReference.rst
===
--- clang/docs/ClangCommandLineReference.rst
+++ clang/docs/ClangCommandLineReference.rst
@@ -1942,8 +1942,9 @@
 
 .. option:: -ftime-trace
 
-Turn on time profiler. Results can be analyzed with chrome://tracing or
-`Speedscope App `_ for flamegraph visualization
+Turn on time profiler. Generates JSON file based on output filename.  Results
+can be analyzed with chrome://tracing or `Speedscope App
+`_ for flamegraph visualization
 
 .. option:: -ftime-trace-granularity=
 


Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -269,8 +269,6 @@
   // FIXME(ibiryukov): make profilerOutput flush in destructor instead.
   profilerOutput->flush();
   llvm::timeTraceProfilerCleanup();
-
-  llvm::errs() << "Time trace json-file dumped to " << Path.str() << "\n";
 }
   }
 
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1793,7 +1793,10 @@
 def fthreadsafe_statics : Flag<["-"], "fthreadsafe-statics">, Group;
 def ftime_report : Flag<["-"], "ftime-report">, Group, Flags<[CC1Option]>;
 def ftime_trace : Flag<["-"], "ftime-trace">, Group,
-  HelpText<"Turn on time profiler">, Flags<[CC1Option, CoreOption]>;
+  HelpText<"Turn on time profiler. Generates JSON file based on output filename. "
+   "Results can be analyzed with chrome://tracing or `Speedscope App "
+   "`_ for flamegraph visualization." >,
+  Flags<[CC1Option, CoreOption]>;
 def ftime_trace_granularity_EQ : Joined<["-"], "ftime-trace-granularity=">, Group,
   HelpText<"Minimum time granularity (in microseconds) traced by time profiler">,
   Flags<[CC1Option, CoreOption]>;
Index: clang/docs/ClangCommandLineReference.rst
===
--- clang/docs/ClangCommandLineReference.rst
+++ clang/docs/ClangCommandLineReference.rst
@@ -1942,8 +1942,9 @@
 
 .. option:: -ftime-trace
 
-Turn on time profiler. Results can be analyzed with chrome://tracing or
-`Speedscope App

[PATCH] D68707: [clang-format] throws an incorrect assertion in consumeToken() formatting the MSVC stl

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 224075.
MyDeveloperDay marked an inline comment as done.
MyDeveloperDay added a comment.

I'm not even sure if the assertion is valid?


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

https://reviews.llvm.org/D68707

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
@@ -14679,6 +14679,12 @@
   */
 }
 
-} // end namespace
-} // end namespace format
-} // end namespace clang
+TEST_F(FormatTest, STLWhileNotDefineChed) {
+  verifyFormat("#if defined(while)\n"
+   "#define while EMIT WARNING C4005\n"
+   "#endif // while");
+}
+
+} // namespace
+} // namespace format
+} // namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -844,7 +844,6 @@
   break;
 case tok::kw_if:
 case tok::kw_while:
-  assert(!Line.startsWith(tok::hash));
   if (Tok->is(tok::kw_if) && CurrentToken &&
   CurrentToken->isOneOf(tok::kw_constexpr, tok::identifier))
 next();


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14679,6 +14679,12 @@
   */
 }
 
-} // end namespace
-} // end namespace format
-} // end namespace clang
+TEST_F(FormatTest, STLWhileNotDefineChed) {
+  verifyFormat("#if defined(while)\n"
+   "#define while EMIT WARNING C4005\n"
+   "#endif // while");
+}
+
+} // namespace
+} // namespace format
+} // namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -844,7 +844,6 @@
   break;
 case tok::kw_if:
 case tok::kw_while:
-  assert(!Line.startsWith(tok::hash));
   if (Tok->is(tok::kw_if) && CurrentToken &&
   CurrentToken->isOneOf(tok::kw_constexpr, tok::identifier))
 next();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68710: Remove time-trace message as it is inconsistent style

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru added inline comments.



Comment at: clang/include/clang/Driver/Options.td:1797
+  HelpText<"Turn on time profiler. Generates JSON file based on output 
filename. "
+   "Results can be analyzed with chrome://tracing or `Speedscope App "
+   "`_ for flamegraph visualization." >,

I am not a big fan of adding products in the in-product help.
If Chrome removes the feature or Speedscope goes done, we will still list that 
in the old version of clang.
Deprecated doc is more common and accepted.

I would remove this line and the next line and keep the doc.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68710



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

[PATCH] D68028: [clang] Add no_builtin attribute

[Guillaume Chatelet via Phabricator via cfe-commits]

gchatelet updated this revision to Diff 224082.
gchatelet added a comment.

- Address aaron ballman comments
- Checks function name validity and errors when passed 0 argument.
- Update documentation and rebase
- Rewrote mergeNoBuiltinAttr
- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68028

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/no-builtin.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/no-builtin.c

Index: clang/test/Sema/no-builtin.c
===
--- /dev/null
+++ clang/test/Sema/no-builtin.c
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - -fsyntax-only -verify %s
+
+void valid_attribute_wildcard() __attribute__((no_builtin("*"))) {}
+void valid_attribute_function() __attribute__((no_builtin("memcpy"))) {}
+void valid_attribute_functions() __attribute__((no_builtin("memcpy"))) __attribute__((no_builtin("memcmp"))) {}
+
+void no_builtin_no_argument() __attribute__((no_builtin)) {}
+// expected-error@-1 {{'no_builtin' attribute takes at least 1 argument}}
+
+void no_builtin_no_argument2() __attribute__((no_builtin())) {}
+// expected-error@-1 {{'no_builtin' attribute takes at least 1 argument}}
+
+void invalid_builtin() __attribute__((no_builtin("not_a_builtin"))) {}
+// expected-error@-1 {{'not_a_builtin' is not a valid function name for no_builtin}}
+
+void wildcard_and_functionname() __attribute__((no_builtin("*", "memcpy"))) {}
+// expected-error@-1 {{no_builtin wildcard (*) cannot be composed with other function names}}
+
+void wildcard_and_functionname2() __attribute__((no_builtin("*"))) __attribute__((no_builtin("memcpy"))) {}
+// expected-error@-1 {{no_builtin wildcard (*) cannot be composed with other function names}}
+
+void nobuiltin_on_declaration() __attribute__((no_builtin("memcpy")));
+// expected-error@-1 {{no_builtin attribute is permitted on definitions only}}
+
+int __attribute__((no_builtin("*"))) variable;
+// expected-warning@-1 {{'no_builtin' attribute only applies to functions}}
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
@@ -74,6 +74,7 @@
 // CHECK-NEXT: NSConsumed (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: NSConsumesSelf (SubjectMatchRule_objc_method)
 // CHECK-NEXT: Naked (SubjectMatchRule_function)
+// CHECK-NEXT: NoBuiltin (SubjectMatchRule_function)
 // CHECK-NEXT: NoCommon (SubjectMatchRule_variable)
 // CHECK-NEXT: NoDebug (SubjectMatchRule_type_alias, SubjectMatchRule_hasType_functionType, SubjectMatchRule_objc_method, SubjectMatchRule_variable_not_is_parameter)
 // CHECK-NEXT: NoDestroy (SubjectMatchRule_variable)
Index: clang/test/CodeGen/no-builtin.c
===
--- /dev/null
+++ clang/test/CodeGen/no-builtin.c
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - %s | FileCheck %s
+
+// CHECK-LABEL: define void @foo_no_mempcy() #0
+void foo_no_mempcy() __attribute__((no_builtin("memcpy"))) {}
+
+// CHECK-LABEL: define void @foo_no_builtins() #1
+void foo_no_builtins() __attribute__((no_builtin("*"))) {}
+
+// CHECK-LABEL: define void @foo_no_mempcy_memset() #2
+void foo_no_mempcy_memset() __attribute__((no_builtin("memset", "memcpy"))) {}
+
+// CHECK-LABEL: define void @separate_attrs() #2
+void separate_attrs() __attribute__((no_builtin("memset"))) __attribute__((no_builtin("memcpy"))) {}
+
+// CHECK-LABEL: define void @wildcard_wins() #1
+void wildcard_wins() __attribute__((no_builtin("memset"))) __attribute__((no_builtin("*"))) __attribute__((no_builtin("memcpy"))) {}
+
+// CHECK: attributes #0 = {{{.*}}"no-builtin-memcpy"{{.*}}}
+// CHECK: attributes #1 = {{{.*}}"no-builtins"{{.*}}}
+// CHECK: attributes #2 = {{{.*}}"no-builtin-memcpy"{{.*}}"no-builtin-memset"{{.*}}}
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -1068,6 +1068,56 @@
   S.Context, AL, Cond, Msg, DiagType, ArgDependent, cast(D)));
 }
 
+NoBuiltinAttr *
+Sema::mergeNoBuiltinAttr(Decl *D, const AttributeCommonInfo &CI,
+ llvm::ArrayRef FunctionNames) {
+  llvm::SmallVector Names;
+  bool HasWildcard = false;
+
+  const auto TestAndPushBack = [&Names, &HasWildcard](StringRef Name) {
+HasWildcard |= (Name == "*");
+Names.push_back(Name)

[PATCH] D68710: Remove time-trace message as it is inconsistent style

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru added a comment.

besides that, I think we should do it; thanks :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68710



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

[PATCH] D68028: [clang] Add no_builtin attribute

[Guillaume Chatelet via Phabricator via cfe-commits]

gchatelet updated this revision to Diff 224085.
gchatelet added a comment.

- Remove leftovers


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68028

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/no-builtin.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/no-builtin.c

Index: clang/test/Sema/no-builtin.c
===
--- /dev/null
+++ clang/test/Sema/no-builtin.c
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - -fsyntax-only -verify %s
+
+void valid_attribute_wildcard() __attribute__((no_builtin("*"))) {}
+void valid_attribute_function() __attribute__((no_builtin("memcpy"))) {}
+void valid_attribute_functions() __attribute__((no_builtin("memcpy"))) __attribute__((no_builtin("memcmp"))) {}
+
+void no_builtin_no_argument() __attribute__((no_builtin)) {}
+// expected-error@-1 {{'no_builtin' attribute takes at least 1 argument}}
+
+void no_builtin_no_argument2() __attribute__((no_builtin())) {}
+// expected-error@-1 {{'no_builtin' attribute takes at least 1 argument}}
+
+void invalid_builtin() __attribute__((no_builtin("not_a_builtin"))) {}
+// expected-error@-1 {{'not_a_builtin' is not a valid function name for no_builtin}}
+
+void wildcard_and_functionname() __attribute__((no_builtin("*", "memcpy"))) {}
+// expected-error@-1 {{no_builtin wildcard (*) cannot be composed with other function names}}
+
+void wildcard_and_functionname2() __attribute__((no_builtin("*"))) __attribute__((no_builtin("memcpy"))) {}
+// expected-error@-1 {{no_builtin wildcard (*) cannot be composed with other function names}}
+
+void nobuiltin_on_declaration() __attribute__((no_builtin("memcpy")));
+// expected-error@-1 {{no_builtin attribute is permitted on definitions only}}
+
+int __attribute__((no_builtin("*"))) variable;
+// expected-warning@-1 {{'no_builtin' attribute only applies to functions}}
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
@@ -74,6 +74,7 @@
 // CHECK-NEXT: NSConsumed (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: NSConsumesSelf (SubjectMatchRule_objc_method)
 // CHECK-NEXT: Naked (SubjectMatchRule_function)
+// CHECK-NEXT: NoBuiltin (SubjectMatchRule_function)
 // CHECK-NEXT: NoCommon (SubjectMatchRule_variable)
 // CHECK-NEXT: NoDebug (SubjectMatchRule_type_alias, SubjectMatchRule_hasType_functionType, SubjectMatchRule_objc_method, SubjectMatchRule_variable_not_is_parameter)
 // CHECK-NEXT: NoDestroy (SubjectMatchRule_variable)
Index: clang/test/CodeGen/no-builtin.c
===
--- /dev/null
+++ clang/test/CodeGen/no-builtin.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - %s | FileCheck %s
+
+// CHECK-LABEL: define void @foo_no_mempcy() #0
+void foo_no_mempcy() __attribute__((no_builtin("memcpy"))) {}
+
+// CHECK-LABEL: define void @foo_no_builtins() #1
+void foo_no_builtins() __attribute__((no_builtin("*"))) {}
+
+// CHECK-LABEL: define void @foo_no_mempcy_memset() #2
+void foo_no_mempcy_memset() __attribute__((no_builtin("memset", "memcpy"))) {}
+
+// CHECK-LABEL: define void @separate_attrs() #2
+void separate_attrs() __attribute__((no_builtin("memset"))) __attribute__((no_builtin("memcpy"))) {}
+
+// CHECK: attributes #0 = {{{.*}}"no-builtin-memcpy"{{.*}}}
+// CHECK: attributes #1 = {{{.*}}"no-builtins"{{.*}}}
+// CHECK: attributes #2 = {{{.*}}"no-builtin-memcpy"{{.*}}"no-builtin-memset"{{.*}}}
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -1068,6 +1068,56 @@
   S.Context, AL, Cond, Msg, DiagType, ArgDependent, cast(D)));
 }
 
+NoBuiltinAttr *
+Sema::mergeNoBuiltinAttr(Decl *D, const AttributeCommonInfo &CI,
+ llvm::ArrayRef FunctionNames) {
+  llvm::SmallVector Names;
+  bool HasWildcard = false;
+
+  const auto TestAndPushBack = [&Names, &HasWildcard](StringRef Name) {
+HasWildcard |= (Name == "*");
+Names.push_back(Name);
+  };
+
+  if (const auto *NBA = D->getAttr())
+for (StringRef FunctionName : NBA->functionNames())
+  TestAndPushBack(FunctionName);
+
+  for (StringRef FunctionName : FunctionNames)
+TestAndPushBack(FunctionName);
+
+  if (HasWildcard && Names.size() > 1)
+Diag(D->getLocation(),
+ diag::err_attribute_no_builtin_wildcar

[PATCH] D66049: [analyzer] PR41729: Fix some false positives and improve strlcat and strlcpy modeling

[Daniel Krupp via Phabricator via cfe-commits]

dkrupp updated this revision to Diff 224088.
dkrupp marked 9 inline comments as done.
dkrupp added a comment.

@Szelethus thanks for your review.
I fixed your suggestions.


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

https://reviews.llvm.org/D66049

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

Index: clang/test/Analysis/bsd-string.c
===
--- clang/test/Analysis/bsd-string.c
+++ clang/test/Analysis/bsd-string.c
@@ -9,6 +9,7 @@
 typedef __typeof(sizeof(int)) size_t;
 size_t strlcpy(char *dst, const char *src, size_t n);
 size_t strlcat(char *dst, const char *src, size_t n);
+size_t strlen(const char *s);
 void clang_analyzer_eval(int);
 
 void f1() {
@@ -18,9 +19,11 @@
 
 void f2() {
   char buf[5];
-  strlcpy(buf, "abcd", sizeof(buf)); // expected-no-warning
-  // FIXME: This should not warn. The string is safely truncated.
-  strlcat(buf, "efgh", sizeof(buf)); // expected-warning{{Size argument is greater than the free space in the destination buffer}}
+  size_t len;
+  len = strlcpy(buf, "abcd", sizeof(buf)); // expected-no-warning
+  clang_analyzer_eval(len == 4); // expected-warning{{TRUE}}
+  len = strlcat(buf, "efgh", sizeof(buf)); // expected-no-warning
+  clang_analyzer_eval(len == 8); // expected-warning{{TRUE}}
 }
 
 void f3() {
@@ -44,7 +47,88 @@
   clang_analyzer_eval(len == 7); // expected-warning{{TRUE}}
 }
 
+
 int f7() {
   char buf[8];
   return strlcpy(buf, "1234567", 0); // no-crash
 }
+
+void f8(){
+  char buf[5];
+  size_t len;
+
+  // basic strlcpy
+  len = strlcpy(buf,"123", sizeof(buf));
+  clang_analyzer_eval(len==3);// expected-warning{{TRUE}}
+  len = strlen(buf);
+  clang_analyzer_eval(len==3);// expected-warning{{TRUE}}
+
+  // testing bounded strlcat
+  len = strlcat(buf,"456", sizeof(buf));
+  clang_analyzer_eval(len==6);// expected-warning{{TRUE}}
+  len = strlen(buf);
+  clang_analyzer_eval(len==4);// expected-warning{{TRUE}}
+
+  // testing strlcat with size==0
+  len = strlcat(buf,"789", 0);
+  clang_analyzer_eval(len==7);// expected-warning{{TRUE}}
+  len = strlen(buf);
+  clang_analyzer_eval(len==4);// expected-warning{{TRUE}}
+
+  // testing strlcpy with size==0
+  len = strlcpy(buf,"123",0);
+  clang_analyzer_eval(len==3);// expected-warning{{TRUE}}
+  len = strlen(buf);
+  clang_analyzer_eval(len==4);// expected-warning{{TRUE}}
+
+}
+
+void f9(int unknown_size, char* unknown_src, char* unknown_dst){
+  char buf[8];
+  size_t len;
+
+  len = strlcpy(buf,"abba",sizeof(buf));
+
+  clang_analyzer_eval(len==4);// expected-warning{{TRUE}}
+  clang_analyzer_eval(strlen(buf)==4);// expected-warning{{TRUE}}
+
+  //size is unknown
+  len = strlcat(buf,"cd", unknown_size);
+  clang_analyzer_eval(len==6);// expected-warning{{TRUE}}
+  clang_analyzer_eval(strlen(buf)>=4);// expected-warning{{TRUE}}
+
+  //dst is unknown
+  len = strlcpy(unknown_dst,"abbc",unknown_size);
+  clang_analyzer_eval(len==4);// expected-warning{{TRUE}}
+  clang_analyzer_eval(strlen(unknown_dst));// expected-warning{{UNKNOWN}}
+
+  //src is unknown
+  len = strlcpy(buf,unknown_src, sizeof(buf));
+  clang_analyzer_eval(len);// expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(strlen(buf));// expected-warning{{UNKNOWN}}
+
+  //src, dst is unknown
+  len = strlcpy(unknown_dst, unknown_src, unknown_size);
+  clang_analyzer_eval(len);// expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(strlen(unknown_dst));// expected-warning{{UNKNOWN}}
+
+  //size is unknown
+  len = strlcat(buf+2,unknown_src+1, sizeof(buf));// expected-warning{{Size argument is greater than the length of the destination buffer}};
+}
+
+void f10(){
+  char buf[8];
+  size_t len;
+
+  len = strlcpy(buf,"abba",sizeof(buf));
+  clang_analyzer_eval(len==4);// expected-warning{{TRUE}}
+  strlcat(buf, "efghi",9);// expected-warning{{Size argument is greater than the length of the destination buffer}}
+}
+
+void f11() {
+  //test for Bug 41729
+  char a[256], b[256];
+  strlcpy(a, "world", sizeof(a));
+  strlcpy(b, "hello ", sizeof(b));
+  strlcat(b, a, sizeof(b)); // no-warning
+}
Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -28,6 +28,7 @@
 using namespace ento;
 
 namespace {
+enum class ConcatFnKind { none = 0, strcat = 1, strlcat = 2 };
 class CStringChecker : public Checker< eval::Call,
  check::PreStmt,
  check::LiveSymbols,
@@ -129,11 +130,8 @@
   void evalStrncpy(CheckerContext &C, const CallExpr *CE) const;
   void evalStpcpy(CheckerContext &C, const CallExpr *CE) const;
   void evalStrlcpy(CheckerContext &C, const CallExpr *CE) const;
-  void evalStrcpyCommon(CheckerContext &C,
-const CallExpr *CE,
-

[PATCH] D66049: [analyzer] PR41729: Fix some false positives and improve strlcat and strlcpy modeling

[Daniel Krupp via Phabricator via cfe-commits]

dkrupp added a comment.

I also analyzed openssl with the baseline and this version, but did not find 
any new warnings.
See:
http://codechecker-demo.eastus.cloudapp.azure.com/Default/#run=D66049_baseline&newcheck=D66049_improved&review-status=Unreviewed&review-status=Confirmed&detection-status=New&detection-status=Reopened&detection-status=Unresolved&tab=D66049_baseline_diff_D66049_improved




Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:1580
 
   // If the function is strncpy, strncat, etc... it is bounded.
   if (isBounded) {

Szelethus wrote:
> Ah, okay, so the assumption is that bounded functions' third argument is 
> always a numerical size parameter. Why isn't that enforced at all?
How should we enforce this?


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

https://reviews.llvm.org/D66049



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

[PATCH] D68707: [clang-format] throws an incorrect assertion in consumeToken() formatting the MSVC stl

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added a comment.

To me, whether or not the assertion was valid is irrelevant. That assertion was 
intentionally added, and its replacement with an `if()` statement materially 
changes the inputs and outputs of the function. I'm questioning whether the 
input of a "while" token within a preprocessor statement should output `true` 
or `false`. (Before, it didn't output anything; it errored.) Does this make 
sense? I'm just asking for clarification on this change.


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

https://reviews.llvm.org/D68707



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

[PATCH] D66049: [analyzer] PR41729: Fix some false positives and improve strlcat and strlcpy modeling

[Daniel Krupp via Phabricator via cfe-commits]

dkrupp updated this revision to Diff 224090.
dkrupp added a comment.

Fixing minor capitalization issue and removing an extra newline.


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

https://reviews.llvm.org/D66049

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

Index: clang/test/Analysis/bsd-string.c
===
--- clang/test/Analysis/bsd-string.c
+++ clang/test/Analysis/bsd-string.c
@@ -9,6 +9,7 @@
 typedef __typeof(sizeof(int)) size_t;
 size_t strlcpy(char *dst, const char *src, size_t n);
 size_t strlcat(char *dst, const char *src, size_t n);
+size_t strlen(const char *s);
 void clang_analyzer_eval(int);
 
 void f1() {
@@ -18,9 +19,11 @@
 
 void f2() {
   char buf[5];
-  strlcpy(buf, "abcd", sizeof(buf)); // expected-no-warning
-  // FIXME: This should not warn. The string is safely truncated.
-  strlcat(buf, "efgh", sizeof(buf)); // expected-warning{{Size argument is greater than the free space in the destination buffer}}
+  size_t len;
+  len = strlcpy(buf, "abcd", sizeof(buf)); // expected-no-warning
+  clang_analyzer_eval(len == 4); // expected-warning{{TRUE}}
+  len = strlcat(buf, "efgh", sizeof(buf)); // expected-no-warning
+  clang_analyzer_eval(len == 8); // expected-warning{{TRUE}}
 }
 
 void f3() {
@@ -48,3 +51,83 @@
   char buf[8];
   return strlcpy(buf, "1234567", 0); // no-crash
 }
+
+void f8(){
+  char buf[5];
+  size_t len;
+
+  // basic strlcpy
+  len = strlcpy(buf,"123", sizeof(buf));
+  clang_analyzer_eval(len==3);// expected-warning{{TRUE}}
+  len = strlen(buf);
+  clang_analyzer_eval(len==3);// expected-warning{{TRUE}}
+
+  // testing bounded strlcat
+  len = strlcat(buf,"456", sizeof(buf));
+  clang_analyzer_eval(len==6);// expected-warning{{TRUE}}
+  len = strlen(buf);
+  clang_analyzer_eval(len==4);// expected-warning{{TRUE}}
+
+  // testing strlcat with size==0
+  len = strlcat(buf,"789", 0);
+  clang_analyzer_eval(len==7);// expected-warning{{TRUE}}
+  len = strlen(buf);
+  clang_analyzer_eval(len==4);// expected-warning{{TRUE}}
+
+  // testing strlcpy with size==0
+  len = strlcpy(buf,"123",0);
+  clang_analyzer_eval(len==3);// expected-warning{{TRUE}}
+  len = strlen(buf);
+  clang_analyzer_eval(len==4);// expected-warning{{TRUE}}
+
+}
+
+void f9(int unknown_size, char* unknown_src, char* unknown_dst){
+  char buf[8];
+  size_t len;
+
+  len = strlcpy(buf,"abba",sizeof(buf));
+
+  clang_analyzer_eval(len==4);// expected-warning{{TRUE}}
+  clang_analyzer_eval(strlen(buf)==4);// expected-warning{{TRUE}}
+
+  //size is unknown
+  len = strlcat(buf,"cd", unknown_size);
+  clang_analyzer_eval(len==6);// expected-warning{{TRUE}}
+  clang_analyzer_eval(strlen(buf)>=4);// expected-warning{{TRUE}}
+
+  //dst is unknown
+  len = strlcpy(unknown_dst,"abbc",unknown_size);
+  clang_analyzer_eval(len==4);// expected-warning{{TRUE}}
+  clang_analyzer_eval(strlen(unknown_dst));// expected-warning{{UNKNOWN}}
+
+  //src is unknown
+  len = strlcpy(buf,unknown_src, sizeof(buf));
+  clang_analyzer_eval(len);// expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(strlen(buf));// expected-warning{{UNKNOWN}}
+
+  //src, dst is unknown
+  len = strlcpy(unknown_dst, unknown_src, unknown_size);
+  clang_analyzer_eval(len);// expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(strlen(unknown_dst));// expected-warning{{UNKNOWN}}
+
+  //size is unknown
+  len = strlcat(buf+2,unknown_src+1, sizeof(buf));// expected-warning{{Size argument is greater than the length of the destination buffer}};
+}
+
+void f10(){
+  char buf[8];
+  size_t len;
+
+  len = strlcpy(buf,"abba",sizeof(buf));
+  clang_analyzer_eval(len==4);// expected-warning{{TRUE}}
+  strlcat(buf, "efghi",9);// expected-warning{{Size argument is greater than the length of the destination buffer}}
+}
+
+void f11() {
+  //test for Bug 41729
+  char a[256], b[256];
+  strlcpy(a, "world", sizeof(a));
+  strlcpy(b, "hello ", sizeof(b));
+  strlcat(b, a, sizeof(b)); // no-warning
+}
Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -28,6 +28,7 @@
 using namespace ento;
 
 namespace {
+enum class ConcatFnKind { none = 0, strcat = 1, strlcat = 2 };
 class CStringChecker : public Checker< eval::Call,
  check::PreStmt,
  check::LiveSymbols,
@@ -129,11 +130,8 @@
   void evalStrncpy(CheckerContext &C, const CallExpr *CE) const;
   void evalStpcpy(CheckerContext &C, const CallExpr *CE) const;
   void evalStrlcpy(CheckerContext &C, const CallExpr *CE) const;
-  void evalStrcpyCommon(CheckerContext &C,
-const CallExpr *CE,
-bool returnEnd,
-bool isBounded,
-bool isAppending

[PATCH] D68715: [mangle] Fix mangling where an extra mangle context is required.

[Michael Liao via Phabricator via cfe-commits]

hliao created this revision.
hliao added reviewers: rsmith, eli.friedman.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

- [Itanium C++ ABI][1], for certain contexts like default parameter and etc., 
mangling numbering will be local to the particular argument in which it appears.
- However, for these cases, the mangle numbering context is allocated per 
expression evaluation stack entry. That causes, for example, two lambdas 
defined/used understand the same default parameter are numbered as the same 
value and, in turn, one of them is not generated at all.
- In this patch, an extra mangle numbering context map is maintained in the AST 
context to map taht extra declaration context to its numbering context. So 
that, 2 different lambdas defined/used in the same default parameter are 
numbered differently.
- Minor coding change to perfer using tuple for multiple return values.

[1]: https://itanium-cxx-abi.github.io/cxx-abi/abi.html


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68715

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/test/CodeGenCXX/mangle-lambdas.cpp

Index: clang/test/CodeGenCXX/mangle-lambdas.cpp
===
--- clang/test/CodeGenCXX/mangle-lambdas.cpp
+++ clang/test/CodeGenCXX/mangle-lambdas.cpp
@@ -178,18 +178,24 @@
 }
 
 namespace std {
-  struct type_info;
+  struct type_info {
+bool before(const type_info &) const noexcept;
+  };
 }
 namespace PR12123 {
   struct A { virtual ~A(); } g;
+  struct C { virtual ~C(); } k;
   struct B {
 void f(const std::type_info& x = typeid([]()->A& { return g; }()));
 void h();
+void j(bool cond = typeid([]() -> A & { return g; }()).before(typeid([]() -> C & { return k; }(;
   };
-  void B::h() { f(); }
+  void B::h() { f(); j(); }
 }
 
 // CHECK-LABEL: define linkonce_odr dereferenceable({{[0-9]+}}) %"struct.PR12123::A"* @_ZZN7PR121231B1fERKSt9type_infoEd_NKUlvE_clEv
+// CHECK-LABEL: define linkonce_odr dereferenceable({{[0-9]+}}) %"struct.PR12123::A"* @_ZZN7PR121231B1jEbEd_NKUlvE_clEv
+// CHECK-LABEL: define linkonce_odr dereferenceable({{[0-9]+}}) %"struct.PR12123::C"* @_ZZN7PR121231B1jEbEd_NKUlvE0_clEv
 
 // CHECK-LABEL: define {{.*}} @_Z{{[0-9]*}}testVarargsLambdaNumberingv(
 inline int testVarargsLambdaNumbering() {
Index: clang/lib/Sema/SemaLambda.cpp
===
--- clang/lib/Sema/SemaLambda.cpp
+++ clang/lib/Sema/SemaLambda.cpp
@@ -272,12 +272,11 @@
   return false;
 }
 
-MangleNumberingContext *
-Sema::getCurrentMangleNumberContext(const DeclContext *DC,
-Decl *&ManglingContextDecl) {
+std::tuple
+Sema::getCurrentMangleNumberContext(const DeclContext *DC) {
   // Compute the context for allocating mangling numbers in the current
   // expression, if the ABI requires them.
-  ManglingContextDecl = ExprEvalContexts.back().ManglingContextDecl;
+  Decl *ManglingContextDecl = ExprEvalContexts.back().ManglingContextDecl;
 
   enum ContextKind {
 Normal,
@@ -325,22 +324,18 @@
 if ((IsInNonspecializedTemplate &&
  !(ManglingContextDecl && isa(ManglingContextDecl))) ||
 isInInlineFunction(CurContext)) {
-  ManglingContextDecl = nullptr;
   while (auto *CD = dyn_cast(DC))
 DC = CD->getParent();
-  return &Context.getManglingNumberContext(DC);
+  return std::make_tuple(&Context.getManglingNumberContext(DC), nullptr);
 }
 
-ManglingContextDecl = nullptr;
-return nullptr;
+return std::make_tuple(nullptr, nullptr);
   }
 
   case StaticDataMember:
 //  -- the initializers of nonspecialized static members of template classes
-if (!IsInNonspecializedTemplate) {
-  ManglingContextDecl = nullptr;
-  return nullptr;
-}
+if (!IsInNonspecializedTemplate)
+  return std::make_tuple(nullptr, nullptr);
 // Fall through to get the current context.
 LLVM_FALLTHROUGH;
 
@@ -352,21 +347,15 @@
 //  -- the initializers of inline variables
   case VariableTemplate:
 //  -- the initializers of templated variables
-return &ExprEvalContexts.back().getMangleNumberingContext(Context);
+return std::make_tuple(
+&Context.getManglingNumberContext(ASTContext::NeedExtraManglingDecl,
+  ManglingContextDecl),
+ManglingContextDecl);
   }
 
   llvm_unreachable("unexpected context");
 }
 
-MangleNumberingContext &
-Sema::ExpressionEvaluationContextRecord::getMangleNumberingContext(
-ASTContext &Ctx) {
-  assert(ManglingContextDecl && "Need to have a context declaration");
-  if (!MangleNumbering)
-MangleNumbering = Ctx.createMangleNumberingContext();
-  return *MangleNumbering;
-}
-
 CXXMethodDecl *Sema::startLambdaDefinition(
 CXXRecordD

[PATCH] D51741: [coro]Pass rvalue reference for named local variable to return_value

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.
Herald added a project: LLVM.

This change breaks the following code that worked before:

  task f(MoveOnly &value) {
co_return value;
  }

The error message is:

  clang/test/SemaCXX/coroutine-rvo.cpp:60:13: error: call to deleted 
constructor of 'MoveOnly'
co_return value;
  ^
  clang/test/SemaCXX/coroutine-rvo.cpp:43:3: note: 'MoveOnly' has been 
explicitly marked deleted here
MoveOnly(const MoveOnly&) = delete;
^

Is that maybe intentional, and is the code not intended to compile?




Comment at: cfe/trunk/lib/Sema/SemaCoroutine.cpp:846
+  if (E) {
+auto NRVOCandidate = this->getCopyElisionCandidate(E->getType(), E, 
CES_AsIfByStdMove);
+if (NRVOCandidate) {

Why not `CES_Strict` like in `Sema::BuildReturnStmt`? With `CES_Strict` the 
test still works, and we can also return references.



Comment at: cfe/trunk/lib/Sema/SemaCoroutine.cpp:849
+  InitializedEntity Entity =
+  InitializedEntity::InitializeResult(Loc, E->getType(), 
NRVOCandidate);
+  ExprResult MoveResult = this->PerformMoveOrCopyInitialization(

The last parameter has type `bool`, and because we're in `if (NRVOCandidate)`, 
that will always be true. Wouldn't it be more straightforward to just pass 
`true` into the function?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D51741



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

[PATCH] D68681: [libc++][test] Miscellaneous MSVC cleanups

[Casey Carter via Phabricator via cfe-commits]

CaseyCarter added inline comments.



Comment at: 
test/std/containers/unord/unord.map/unord.map.cnstr/deduct.pass.cpp:70
+#pragma warning(disable: 4244) // '%s': conversion from '%s' to '%s', possible 
loss of data
+#endif // TEST_COMPILER_C1XX
+

Quuxplusone wrote:
> Alternatively, if you wanted to change the instances of `hash` to 
> `hash<>`, that would be fine with me. (Just as long as it's something 
> distinguishable from the default of `hash`.)
I do like the idea of avoiding rather than suppressing the warning. I'll change 
these tests to use `hash` instead of `hash`, which avoids 
truncating conversions and preserves the "neither `int` nor `long`" property 
which provides some assurance the deduction guides aren't getting the type from 
the key type or mapped type.


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

https://reviews.llvm.org/D68681



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

[PATCH] D68681: [libc++][test] Miscellaneous MSVC cleanups

[Casey Carter via Phabricator via cfe-commits]

CaseyCarter updated this revision to Diff 224092.
CaseyCarter edited the summary of this revision.
CaseyCarter added a comment.

Avoid rather than suppress truncation warnings in `unordered_meow` deduction 
guide tests.


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

https://reviews.llvm.org/D68681

Files:
  test/std/containers/associative/map/map.cons/assign_initializer_list.pass.cpp
  test/std/containers/associative/set/set.cons/assign_initializer_list.pass.cpp
  test/std/containers/unord/unord.map/unord.map.cnstr/deduct.pass.cpp
  test/std/containers/unord/unord.multiset/unord.multiset.cnstr/deduct.pass.cpp
  test/std/containers/unord/unord.set/unord.set.cnstr/deduct.pass.cpp
  test/std/numerics/c.math/abs.pass.cpp
  test/std/numerics/numeric.ops/numeric.ops.midpoint/midpoint.float.pass.cpp
  test/std/utilities/meta/meta.trans/meta.trans.other/underlying_type.pass.cpp

Index: test/std/utilities/meta/meta.trans/meta.trans.other/underlying_type.pass.cpp
===
--- test/std/utilities/meta/meta.trans/meta.trans.other/underlying_type.pass.cpp
+++ test/std/utilities/meta/meta.trans/meta.trans.other/underlying_type.pass.cpp
@@ -42,7 +42,7 @@
 ASSERT_SAME_TYPE(Expected, typename std::underlying_type::type);
 #if TEST_STD_VER > 11
 ASSERT_SAME_TYPE(Expected, typename std::underlying_type_t);
-#endif  
+#endif
 }
 
 enum E { V = INT_MIN };
@@ -79,7 +79,9 @@
 //  SFINAE-able underlying_type
 #if TEST_STD_VER > 17
 static_assert( has_type_member::value, "");
+#ifdef TEST_UNSIGNED_UNDERLYING_TYPE
 static_assert( has_type_member::value, "");
+#endif // TEST_UNSIGNED_UNDERLYING_TYPE
 static_assert( has_type_member::value, "");
 
 static_assert(!has_type_member::value, "");
Index: test/std/numerics/numeric.ops/numeric.ops.midpoint/midpoint.float.pass.cpp
===
--- test/std/numerics/numeric.ops/numeric.ops.midpoint/midpoint.float.pass.cpp
+++ test/std/numerics/numeric.ops/numeric.ops.midpoint/midpoint.float.pass.cpp
@@ -41,7 +41,7 @@
 
 constexpr T maxV = std::numeric_limits::max();
 constexpr T minV = std::numeric_limits::min();
-
+
 //  Things that can be compared exactly
 static_assert((std::midpoint(T(0), T(0))   == T(0)),   "");
 static_assert((std::midpoint(T(2), T(4))   == T(3)),   "");
@@ -58,7 +58,7 @@
 assert((fptest_close_pct(std::midpoint(T(0.1),  T(0.4)),  T(0.25), pct)));
 
 assert((fptest_close_pct(std::midpoint(T(11.2345), T(14.5432)), T(12.5),  pct)));
-
+
 //  From e to pi
 assert((fptest_close_pct(std::midpoint(T(2.71828182845904523536028747135266249775724709369995),
   T(3.14159265358979323846264338327950288419716939937510)),
@@ -86,7 +86,7 @@
 //  TODO
 
 //  Check two values "close to each other"
-T d1 = 3.14;
+T d1 = T(3.14);
 T d0 = std::nextafter(d1, T(2));
 T d2 = std::nextafter(d1, T(5));
 assert(d0 < d1);  // sanity checking
Index: test/std/numerics/c.math/abs.pass.cpp
===
--- test/std/numerics/c.math/abs.pass.cpp
+++ test/std/numerics/c.math/abs.pass.cpp
@@ -9,6 +9,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "test_macros.h"
@@ -75,4 +76,3 @@
 
 return 0;
 }
-
Index: test/std/containers/unord/unord.set/unord.set.cnstr/deduct.pass.cpp
===
--- test/std/containers/unord/unord.set/unord.set.cnstr/deduct.pass.cpp
+++ test/std/containers/unord/unord.set/unord.set.cnstr/deduct.pass.cpp
@@ -78,37 +78,37 @@
 
 {
 const int arr[] = { 1, 2, 1, INT_MAX, 3 };
-std::unordered_set s(std::begin(arr), std::end(arr), 42, std::hash());
+std::unordered_set s(std::begin(arr), std::end(arr), 42, std::hash());
 
-ASSERT_SAME_TYPE(decltype(s), std::unordered_set>);
+ASSERT_SAME_TYPE(decltype(s), std::unordered_set>);
 assert(std::is_permutation(s.begin(), s.end(), std::begin(expected_s), std::end(expected_s)));
 }
 
 {
 const int arr[] = { 1, 2, 1, INT_MAX, 3 };
-std::unordered_set s(std::begin(arr), std::end(arr), 42, std::hash(), test_allocator(0, 40));
+std::unordered_set s(std::begin(arr), std::end(arr), 42, std::hash(), test_allocator(0, 40));
 
-ASSERT_SAME_TYPE(decltype(s), std::unordered_set, std::equal_to, test_allocator>);
+ASSERT_SAME_TYPE(decltype(s), std::unordered_set, std::equal_to, test_allocator>);
 assert(std::is_permutation(s.begin(), s.end(), std::begin(expected_s), std::end(expected_s)));
 assert(s.get_allocator().get_id() == 40);
 }
 
 {
-std::unordered_set, std::equal_to<>, test_allocator> source;
+std::unordered_set, std::equal_to<>, test_allocator> source;
 std::unordered_set s(source);
 ASSERT_SAME_TYPE(decltype(s), decltype(source));
 assert(s.size() == 0);
 }
 
 {
-st

[PATCH] D68683: ARM] Fix arm_neon.h with -flax-vector-conversions=none

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

rsmith accepted this revision.
rsmith added a comment.

Thank you!

We also have a hack in the ARM and AArch64 target info to set the default for 
lax vector conversions back to "all" when NEON is enabled that can now be 
removed.


Repository:
  rC Clang

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

https://reviews.llvm.org/D68683



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

[PATCH] D68694: Do not show "use of a signed integer operand with a binary bitwise operator" for positive integer operands

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: aaron.ballman.
aaron.ballman added a comment.

> In my opinion this check should be disabled in case of integer literals, 
> since there are a lot of existing code (even in system libraries) where user 
> can do nothing, e.g.:

I think that this check should behave how the coding standard dictates it 
behaves. By my reading of HIC++, the check is behaving by design. I think there 
are a few ways forward though (not mutually exclusive):

0) See if the HIC++ folks are interested in clarifying their standard for 
integer literals and if they are, modify the check

1. Add an off-by-default option that suppresses the diagnostic with positive 
integer literals so that users can explicitly decide to deviate locally
2. Document that this happens and is expected behavior; users can disable the 
check if they don't wish to conform to that standard.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68694



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

[PATCH] D68681: [libc++][test] Miscellaneous MSVC cleanups

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

Quuxplusone added inline comments.



Comment at: 
test/std/containers/unord/unord.map/unord.map.cnstr/deduct.pass.cpp:70
+#pragma warning(disable: 4244) // '%s': conversion from '%s' to '%s', possible 
loss of data
+#endif // TEST_COMPILER_C1XX
+

CaseyCarter wrote:
> Quuxplusone wrote:
> > Alternatively, if you wanted to change the instances of `hash` to 
> > `hash<>`, that would be fine with me. (Just as long as it's something 
> > distinguishable from the default of `hash`.)
> I do like the idea of avoiding rather than suppressing the warning. I'll 
> change these tests to use `hash` instead of `hash`, which 
> avoids truncating conversions and preserves the "neither `int` nor `long`" 
> property which provides some assurance the deduction guides aren't getting 
> the type from the key type or mapped type.
Awesome. The CTAD tests LGTM! No opinion on the rest.

(It occurs to me that CTAD makes `std::hash()` mean `std::hash()`, so 
maybe I should have avoided `hash` and `equal_to` in these tests to 
begin with.)


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

https://reviews.llvm.org/D68681



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

[PATCH] D68707: [clang-format] throws an incorrect assertion in consumeToken() formatting the MSVC stl

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:847
 case tok::kw_while:
-  assert(!Line.startsWith(tok::hash));
-  if (Tok->is(tok::kw_if) && CurrentToken &&
-  CurrentToken->isOneOf(tok::kw_constexpr, tok::identifier))
-next();
-  if (CurrentToken && CurrentToken->is(tok::l_paren)) {
-next();
-if (!parseParens(/*LookForDecls=*/true))
-  return false;
+  if (!Line.startsWith(tok::hash)) {
+if (Tok->is(tok::kw_if) && CurrentToken &&

mitchell-stellar wrote:
> It's not clear to me whether or not the token should be consumed. The 
> previous assertion leads me to think no, and in that case, I think this 
> should be
> ```
> if (Line.startsWith(tok::hash))
> return false;
> ```
> A comment on this would also be helpful.
to be honest I'm not sure what the assertion is trying to actually assert (I'm 
not a massive fan of assertions especially like this), just because I see while 
am I not expecting to see a # at the beginning of the line?

I've tried both yours, mine and removing it completely and none of the test 
fail, so I feel like imposing the "Beyonce rule!!" and getting rid of it.


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

https://reviews.llvm.org/D68707



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

[PATCH] D68581: Include leading attributes in DeclStmt's SourceRange

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I think this seems like reasonable behavior (for instance, we include the 
location of a storage class specifier already), but I am curious if @rsmith 
agrees.

> Is there a good way to write a test for this?

Yes, you can put an AST dumping test into the test\AST directory to check that 
you get expected source locations.




Comment at: clang/lib/Parse/ParseStmt.cpp:224
+  if (Attrs.Range.getBegin().isValid())
+  DeclStart = Attrs.Range.getBegin();
   return Actions.ActOnDeclStmt(Decl, DeclStart, DeclEnd);

Indentation looks incorrect here.


Repository:
  rC Clang

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

https://reviews.llvm.org/D68581



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

r374189 - [WebAssembly] Add builtin and intrinsic for v8x16.swizzle

[Thomas Lively via cfe-commits]

Author: tlively
Date: Wed Oct  9 10:45:47 2019
New Revision: 374189

URL: http://llvm.org/viewvc/llvm-project?rev=374189&view=rev
Log:
[WebAssembly] Add builtin and intrinsic for v8x16.swizzle

Summary:
This clang builtin and corresponding LLVM intrinsic are necessary to
expose the exact semantics of the underlying WebAssembly instruction
to users. LLVM produces a poison value if the dynamic swizzle indices
are greater than the vector size, but the WebAssembly instruction sets
the corresponding output lane to zero. Users who depend on this
behavior can safely use this builtin.

Depends on D68527.

Reviewers: aheejin, dschuff

Subscribers: sbc100, jgravelle-google, hiraditya, sunfish, cfe-commits, 
llvm-commits

Tags: #clang, #llvm

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

Modified:
cfe/trunk/include/clang/Basic/BuiltinsWebAssembly.def
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/test/CodeGen/builtins-wasm.c

Modified: cfe/trunk/include/clang/Basic/BuiltinsWebAssembly.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsWebAssembly.def?rev=374189&r1=374188&r2=374189&view=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsWebAssembly.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsWebAssembly.def Wed Oct  9 10:45:47 
2019
@@ -60,6 +60,8 @@ TARGET_BUILTIN(__builtin_wasm_trunc_satu
 TARGET_BUILTIN(__builtin_wasm_trunc_saturate_u_i64_f64, "LLid", "nc", 
"nontrapping-fptoint")
 
 // SIMD builtins
+TARGET_BUILTIN(__builtin_wasm_swizzle_v8x16, "V16cV16cV16c", "nc", 
"unimplemented-simd128")
+
 TARGET_BUILTIN(__builtin_wasm_extract_lane_s_i8x16, "iV16cIi", "nc", "simd128")
 TARGET_BUILTIN(__builtin_wasm_extract_lane_u_i8x16, "iV16cIi", "nc", 
"unimplemented-simd128")
 TARGET_BUILTIN(__builtin_wasm_extract_lane_s_i16x8, "iV8sIi", "nc", "simd128")

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=374189&r1=374188&r2=374189&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Wed Oct  9 10:45:47 2019
@@ -14066,6 +14066,12 @@ Value *CodeGenFunction::EmitWebAssemblyB
  ConvertType(E->getType()));
 return Builder.CreateCall(Callee, {LHS, RHS});
   }
+  case WebAssembly::BI__builtin_wasm_swizzle_v8x16: {
+Value *Src = EmitScalarExpr(E->getArg(0));
+Value *Indices = EmitScalarExpr(E->getArg(1));
+Function *Callee = CGM.getIntrinsic(Intrinsic::wasm_swizzle);
+return Builder.CreateCall(Callee, {Src, Indices});
+  }
   case WebAssembly::BI__builtin_wasm_extract_lane_s_i8x16:
   case WebAssembly::BI__builtin_wasm_extract_lane_u_i8x16:
   case WebAssembly::BI__builtin_wasm_extract_lane_s_i16x8:

Modified: cfe/trunk/test/CodeGen/builtins-wasm.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/builtins-wasm.c?rev=374189&r1=374188&r2=374189&view=diff
==
--- cfe/trunk/test/CodeGen/builtins-wasm.c (original)
+++ cfe/trunk/test/CodeGen/builtins-wasm.c Wed Oct  9 10:45:47 2019
@@ -157,7 +157,6 @@ double max_f64(double x, double y) {
   // WEBASSEMBLY-NEXT: ret
 }
 
-
 int extract_lane_s_i8x16(i8x16 v) {
   return __builtin_wasm_extract_lane_s_i8x16(v, 13);
   // MISSING-SIMD: error: '__builtin_wasm_extract_lane_s_i8x16' needs target 
feature simd128
@@ -539,3 +538,9 @@ i32x4 widen_high_u_i32x4_i16x8(i16x8 v)
   // WEBASSEMBLY: call <4 x i32> @llvm.wasm.widen.high.unsigned.v4i32.v8i16(<8 
x i16> %v)
   // WEBASSEMBLY: ret
 }
+
+i8x16 swizzle_v8x16(i8x16 x, i8x16 y) {
+  return __builtin_wasm_swizzle_v8x16(x, y);
+  // WEBASSEMBLY: call <16 x i8> @llvm.wasm.swizzle(<16 x i8> %x, <16 x i8> %y)
+  // WEBASSEMBLY-NEXT: ret
+}


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

[PATCH] D68531: [WebAssembly] Add builtin and intrinsic for v8x16.swizzle

[Thomas Lively via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3419e90dc1a2: [WebAssembly] Add builtin and intrinsic for 
v8x16.swizzle (authored by tlively).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68531

Files:
  clang/include/clang/Basic/BuiltinsWebAssembly.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-wasm.c
  llvm/include/llvm/IR/IntrinsicsWebAssembly.td
  llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
  llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll

Index: llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
===
--- llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
+++ llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
@@ -11,6 +11,16 @@
 ; ==
 ; 16 x i8
 ; ==
+; CHECK-LABEL: swizzle_v16i8:
+; SIMD128-NEXT: .functype swizzle_v16i8 (v128, v128) -> (v128){{$}}
+; SIMD128-NEXT: v8x16.swizzle $push[[R:[0-9]+]]=, $0, $1{{$}}
+; SIMD128-NEXT: return $pop[[R]]{{$}}
+declare <16 x i8> @llvm.wasm.swizzle(<16 x i8>, <16 x i8>)
+define <16 x i8> @swizzle_v16i8(<16 x i8> %x, <16 x i8> %y) {
+  %a = call <16 x i8> @llvm.wasm.swizzle(<16 x i8> %x, <16 x i8> %y)
+  ret <16 x i8> %a
+}
+
 ; CHECK-LABEL: add_sat_s_v16i8:
 ; SIMD128-NEXT: .functype add_sat_s_v16i8 (v128, v128) -> (v128){{$}}
 ; SIMD128-NEXT: i8x16.add_saturate_s $push[[R:[0-9]+]]=, $0, $1{{$}}
Index: llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
===
--- llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
+++ llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
@@ -278,12 +278,16 @@
 // Swizzle lanes: v8x16.swizzle
 def wasm_swizzle_t : SDTypeProfile<1, 2, []>;
 def wasm_swizzle : SDNode<"WebAssemblyISD::SWIZZLE", wasm_swizzle_t>;
+let Predicates = [HasUnimplementedSIMD128] in
 defm SWIZZLE :
   SIMD_I<(outs V128:$dst), (ins V128:$src, V128:$mask), (outs), (ins),
  [(set (v16i8 V128:$dst),
(wasm_swizzle (v16i8 V128:$src), (v16i8 V128:$mask)))],
  "v8x16.swizzle\t$dst, $src, $mask", "v8x16.swizzle", 192>;
 
+def : Pat<(int_wasm_swizzle (v16i8 V128:$src), (v16i8 V128:$mask)),
+  (SWIZZLE V128:$src, V128:$mask)>;
+
 // Create vector with identical lanes: splat
 def splat2 : PatFrag<(ops node:$x), (build_vector node:$x, node:$x)>;
 def splat4 : PatFrag<(ops node:$x), (build_vector
Index: llvm/include/llvm/IR/IntrinsicsWebAssembly.td
===
--- llvm/include/llvm/IR/IntrinsicsWebAssembly.td
+++ llvm/include/llvm/IR/IntrinsicsWebAssembly.td
@@ -89,6 +89,10 @@
 // SIMD intrinsics
 //===--===//
 
+def int_wasm_swizzle :
+  Intrinsic<[llvm_v16i8_ty],
+[llvm_v16i8_ty, llvm_v16i8_ty],
+[IntrNoMem, IntrSpeculatable]>;
 def int_wasm_sub_saturate_signed :
   Intrinsic<[llvm_anyvector_ty],
 [LLVMMatchType<0>, LLVMMatchType<0>],
Index: clang/test/CodeGen/builtins-wasm.c
===
--- clang/test/CodeGen/builtins-wasm.c
+++ clang/test/CodeGen/builtins-wasm.c
@@ -157,7 +157,6 @@
   // WEBASSEMBLY-NEXT: ret
 }
 
-
 int extract_lane_s_i8x16(i8x16 v) {
   return __builtin_wasm_extract_lane_s_i8x16(v, 13);
   // MISSING-SIMD: error: '__builtin_wasm_extract_lane_s_i8x16' needs target feature simd128
@@ -539,3 +538,9 @@
   // WEBASSEMBLY: call <4 x i32> @llvm.wasm.widen.high.unsigned.v4i32.v8i16(<8 x i16> %v)
   // WEBASSEMBLY: ret
 }
+
+i8x16 swizzle_v8x16(i8x16 x, i8x16 y) {
+  return __builtin_wasm_swizzle_v8x16(x, y);
+  // WEBASSEMBLY: call <16 x i8> @llvm.wasm.swizzle(<16 x i8> %x, <16 x i8> %y)
+  // WEBASSEMBLY-NEXT: ret
+}
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -14066,6 +14066,12 @@
  ConvertType(E->getType()));
 return Builder.CreateCall(Callee, {LHS, RHS});
   }
+  case WebAssembly::BI__builtin_wasm_swizzle_v8x16: {
+Value *Src = EmitScalarExpr(E->getArg(0));
+Value *Indices = EmitScalarExpr(E->getArg(1));
+Function *Callee = CGM.getIntrinsic(Intrinsic::wasm_swizzle);
+return Builder.CreateCall(Callee, {Src, Indices});
+  }
   case WebAssembly::BI__builtin_wasm_extract_lane_s_i8x16:
   case WebAssembly::BI__builtin_wasm_extract_lane_u_i8x16:
   case WebAssembly::BI__builtin_wasm_extract_lane_s_i16x8:
Index: clang/include/clang/Basic/BuiltinsWebAssembly.def
===
--- clang/include/clang/Basic/BuiltinsWebAssembly.def
+++ clang/include/cl

[PATCH] D55125: [clang-tidy] Fix a false positive in misc-redundant-expression check

[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!


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

https://reviews.llvm.org/D55125



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

[PATCH] D68665: [HIP] Fix -save-temps

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 8 inline comments as done.
yaxunl added inline comments.



Comment at: lib/Driver/Driver.cpp:4400
+  // HIP image for device compilation with -fno-gpu-rdc is per compilation
+  // unit.
+  bool IsHIPNoRDC = JA.getOffloadingDeviceKind() == Action::OFK_HIP &&

tra wrote:
> ... so we derive the file name from the main file name.
for -fno-gpu-rdc, there is only one file. We derive the name from that file. In 
-fgpu-rdc case, the file name is determined by the output file name elsewhere.



Comment at: lib/Driver/Driver.cpp:4405
+  if (IsHIPNoRDC)
+Output = BaseName.substr(0, BaseName.rfind('.'));
   Output += OffloadingPrefix;

tra wrote:
> `llvm::sys::path::replace_extension(Output, "");` ?
will do when committing



Comment at: lib/Driver/ToolChains/HIP.cpp:253
+constructLlcCommand(C, JA, Inputs, Args, SubArchName, Prefix, OptCommand,
+true);
   const char *LlcCommand =

tra wrote:
> `/*OutputIsAsm=*/true`
will do



Comment at: lib/Driver/ToolChains/HIP.h:62
+  const char *InputFileName,
+  bool IsAsm = false) const;
 

tra wrote:
> `OutputIsAsm`?  Otherwise it's not quite clear what `IsAsm` refers to.
will do


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

https://reviews.llvm.org/D68665



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

[PATCH] D68581: Include leading attributes in DeclStmt's SourceRange

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

rsmith added a comment.

In D68581#1701787 , @aaron.ballman 
wrote:

> I think this seems like reasonable behavior (for instance, we include the 
> location of a storage class specifier already), but I am curious if @rsmith 
> agrees.


Yes, I do agree. Though please be careful about the case where the first 
attribute occurs after the first decl-specifier (`int __attribute__((...)) x;`).


Repository:
  rC Clang

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

https://reviews.llvm.org/D68581



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

[PATCH] D68716: [clang] prevent crash for nonnull attribut in constant context (Bug 43601)

[Tyker via Phabricator via cfe-commits]

Tyker created this revision.
Tyker added a reviewer: rnk.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

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


Repository:
  rC Clang

https://reviews.llvm.org/D68716

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/attr-nonnull.cpp


Index: clang/test/SemaCXX/attr-nonnull.cpp
===
--- clang/test/SemaCXX/attr-nonnull.cpp
+++ clang/test/SemaCXX/attr-nonnull.cpp
@@ -77,10 +77,11 @@
 constexpr int i32 = f3(0, &c);
 
 __attribute__((nonnull(4))) __attribute__((nonnull)) //expected-error {{out of 
bounds}}
-constexpr int f4(const int*, const int*) {
+constexpr int f4(const int*, const int*, int) {
   return 0;
 }
-constexpr int i4 = f4(&c, 0); //expected-error {{constant expression}} 
expected-note {{null passed}}
-constexpr int i42 = f4(0, &c); //expected-error {{constant expression}} 
expected-note {{null passed}}
+constexpr int i4 = f4(&c, 0, 0); //expected-error {{constant expression}} 
expected-note {{null passed}}
+constexpr int i42 = f4(0, &c, 1); //expected-error {{constant expression}} 
expected-note {{null passed}}
+constexpr int i43 = f4(&c, &c, 0);
 
 }
\ No newline at end of file
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -5451,6 +5451,7 @@
   Success = false;
 } else if (!ForbiddenNullArgs.empty() &&
ForbiddenNullArgs[I - Args.begin()] &&
+   ArgValues[I - Args.begin()].isLValue() &&
ArgValues[I - Args.begin()].isNullPointer()) {
   Info.CCEDiag(*I, diag::note_non_null_attribute_failed);
   if (!Info.noteFailure())


Index: clang/test/SemaCXX/attr-nonnull.cpp
===
--- clang/test/SemaCXX/attr-nonnull.cpp
+++ clang/test/SemaCXX/attr-nonnull.cpp
@@ -77,10 +77,11 @@
 constexpr int i32 = f3(0, &c);
 
 __attribute__((nonnull(4))) __attribute__((nonnull)) //expected-error {{out of bounds}}
-constexpr int f4(const int*, const int*) {
+constexpr int f4(const int*, const int*, int) {
   return 0;
 }
-constexpr int i4 = f4(&c, 0); //expected-error {{constant expression}} expected-note {{null passed}}
-constexpr int i42 = f4(0, &c); //expected-error {{constant expression}} expected-note {{null passed}}
+constexpr int i4 = f4(&c, 0, 0); //expected-error {{constant expression}} expected-note {{null passed}}
+constexpr int i42 = f4(0, &c, 1); //expected-error {{constant expression}} expected-note {{null passed}}
+constexpr int i43 = f4(&c, &c, 0);
 
 }
\ No newline at end of file
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -5451,6 +5451,7 @@
   Success = false;
 } else if (!ForbiddenNullArgs.empty() &&
ForbiddenNullArgs[I - Args.begin()] &&
+   ArgValues[I - Args.begin()].isLValue() &&
ArgValues[I - Args.begin()].isNullPointer()) {
   Info.CCEDiag(*I, diag::note_non_null_attribute_failed);
   if (!Info.noteFailure())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68665: [HIP] Fix -save-temps

[Aaron Enye Shi via Phabricator via cfe-commits]

ashi1 accepted this revision.
ashi1 added a comment.

LGTM


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

https://reviews.llvm.org/D68665



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

[PATCH] D68707: [clang-format] throws an incorrect assertion in consumeToken() formatting the MSVC stl

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar accepted this revision.
mitchell-stellar added a comment.
This revision is now accepted and ready to land.

Okay.

LGTM.


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

https://reviews.llvm.org/D68707



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

[PATCH] D63978: Clang Interface Stubs merger plumbing for Driver

[Ali Tamur via Phabricator via cfe-commits]

tamur added a comment.

It seems that with this patch, llvm-ifs starts to depend on yaml2obj, which as 
far as I know, was only used for testing purposes until now. Is this intended?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63978



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

[PATCH] D68551: [clang-format] [NFC] Ensure clang-format is itself clang-formatted.

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added a comment.

In D68551#1697842 , @mitchell-stellar 
wrote:

> I agree that a system in place that either enforces clang-formatting on 
> commit or after the fact would be ideal. Otherwise, I don't see a need to 
> have to approve these NFC commits.


The current coding policy 
 contains "Our long 
term goal is for the entire codebase to follow the convention, but we 
explicitly do not want patches that do large-scale reformatting of existing 
code." that was added after someone removed all trailing whitespace all LLVM 
files. Reformatting the code you are going to work in is fine, but not on the 
entire code base. Ideally we'd also run the regression tests in a pre-commit 
hook.

Btw, I am the author of the CMakeLists snippet quoted by @MyDeveloperDay. 
Before that, it was a shell script that didn't run on Windows. Making it part 
of the regression test basically eliminated all discussion about code 
formatting, but we had to run large-scale reformatting whenever clang-format 
changed in some way. It also runs by the polly-* buildbots which I personally 
do not like since I don't see code formatting as a reason why a build should 
fail.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68551



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

[PATCH] D68715: [mangle] Fix mangling where an extra mangle context is required.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

The refactoring of the getCurrentMangleNumberContext API mixed with functional 
changes makes this much harder to review quickly.  Please separate it out.




Comment at: clang/lib/Sema/SemaLambda.cpp:353
+  ManglingContextDecl),
+ManglingContextDecl);
   }

If I'm following correctly, the core of the change is right here?  Instead of 
grabbing a context from the ExpressionEvaluationContextRecord directly, we 
instead grab the current decl from it, and look up the context from that?  That 
makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68715



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

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille created this revision.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya, dschuff.
Herald added projects: clang, LLVM.

Implement protection against the stack clash attack [0].

Probe stack allocation every PAGE_SIZE during frame lowering or dynamic 
allocation to make sure the  page guard, if any, is touched when touching the 
stack, in a similar manner to GCC[1].

If possible, use MOV already present in the entry block instead of generating 
new ones.

Only implemented for x86.

[0] https://www.qualys.com/2017/06/19/stack-clash/stack-clash.txt
[1] https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00556.html


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68720

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/stack-clash-protection.c
  clang/test/Driver/stack-clash-protection.c
  llvm/include/llvm/CodeGen/CommandFlags.inc
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/Target/X86/X86FrameLowering.cpp
  llvm/lib/Target/X86/X86FrameLowering.h
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/lib/Target/X86/X86InstrCompiler.td
  llvm/lib/Target/X86/X86InstrInfo.td
  llvm/test/CodeGen/X86/stack-clash-dynamic-alloca.ll
  llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
  llvm/test/CodeGen/X86/stack-clash-medium.ll
  llvm/test/CodeGen/X86/stack-clash-small.ll

Index: llvm/test/CodeGen/X86/stack-clash-small.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-small.ll
@@ -0,0 +1,17 @@
+; RUN: llc --stack-clash-protector < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK-LABEL: foo:
+define i32 @foo() local_unnamed_addr {
+; only one stack growth
+; CHECK: subq
+; CHECK-NOT: subq
+  %a = alloca i32, i64 100, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 98
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
Index: llvm/test/CodeGen/X86/stack-clash-medium.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-medium.ll
@@ -0,0 +1,19 @@
+; RUN: llc --stack-clash-protector < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK-LABEL: foo:
+define i32 @foo() local_unnamed_addr {
+; two stack growth, with a natural probe inbetween and an extra probe afterward
+; CHECK: subq
+; CHECK: movl
+; CHECK: subq
+; CHECK: movq $0
+  %a = alloca i32, i64 2000, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 1198
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
Index: llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
@@ -0,0 +1,23 @@
+; RUN: llc --stack-clash-protector < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK-LABEL: foo:
+define i32 @foo() local_unnamed_addr {
+; two stack growth, with an extra probe inbetween and a natural probe afterward
+; CHECK: subq
+; CHECK-NOT: movq $0
+; CHECK: mov
+; CHECK: subq
+; CHECK-NOT: movq $0
+; CHECK: mov
+  %a = alloca i32, i64 2000, align 16
+  %b0 = getelementptr inbounds i32, i32* %a, i64 98
+  %b1 = getelementptr inbounds i32, i32* %a, i64 1198
+  store volatile i32 1, i32* %b0
+  store volatile i32 1, i32* %b1
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
Index: llvm/test/CodeGen/X86/stack-clash-dynamic-alloca.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-dynamic-alloca.ll
@@ -0,0 +1,16 @@
+; RUN: llc --stack-clash-protector < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK-LABEL: foo:
+define i32 @foo(i32 %n) local_unnamed_addr {
+; CHECK: subq   $4096, %rsp
+; CHECK: movq   $0, (%rsp)
+  %a = alloca i32, i32 %n, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 1198
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
Index: llvm/lib/Target/X86/X86InstrInfo.td
===
--- llvm/lib/Target/X86/X86InstrInfo.td
+++ llvm/lib/Target/X86/X86InstrInfo.td
@@ -121,6 +121,8 @@
 
 def SDT_X86SEG_ALLOCA : SD

[PATCH] D63978: Clang Interface Stubs merger plumbing for Driver

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi marked 3 inline comments as done.
plotfi added a comment.

In D63978#1701864 , @tamur wrote:

> It seems that with this patch, llvm-ifs starts to depend on yaml2obj, which 
> as far as I know, was only used for testing purposes until now. Is this 
> intended?


No not with this patch, but with an earlier patch llvm-ifs does use yaml to 
generate elf. The library was available and elfabi wasn’t. I can move llvm-ifs 
to elfabi when it is ready.




Comment at: clang/lib/Driver/Driver.cpp:3372
+  if (Phase == phases::IfsMerge) {
+assert(Phase == PL.back() && "merging must be final compilation 
step.");
+MergerInputs.push_back(Current);

compnerd wrote:
> plotfi wrote:
> > compnerd wrote:
> > > Does the interface merging have to be the last step?  I could see 
> > > interface merging preceding linking just fine.
> > For now I think that's the expedient thing to do. Do you want to change 
> > that?
> Add a TODO perhaps?
I agree with that. 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63978



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

r374198 - [HIP] Fix -save-temps

[Yaxun Liu via cfe-commits]

Author: yaxunl
Date: Wed Oct  9 11:46:43 2019
New Revision: 374198

URL: http://llvm.org/viewvc/llvm-project?rev=374198&view=rev
Log:
[HIP] Fix -save-temps

Currently clang does not save some of the intermediate file generated during 
device compilation for HIP when -save-temps is specified.

This patch fixes that.

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

Added:
cfe/trunk/test/Driver/hip-save-temps.hip
Modified:
cfe/trunk/lib/Driver/Driver.cpp
cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
cfe/trunk/lib/Driver/ToolChains/HIP.cpp
cfe/trunk/lib/Driver/ToolChains/HIP.h

Modified: cfe/trunk/lib/Driver/Driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=374198&r1=374197&r2=374198&view=diff
==
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ cfe/trunk/lib/Driver/Driver.cpp Wed Oct  9 11:46:43 2019
@@ -4419,11 +4419,22 @@ const char *Driver::GetNamedOutputPath(C
   MakeCLOutputFilename(C.getArgs(), "", BaseName, types::TY_Image);
 } else {
   SmallString<128> Output(getDefaultImageName());
+  // HIP image for device compilation with -fno-gpu-rdc is per compilation
+  // unit.
+  bool IsHIPNoRDC = JA.getOffloadingDeviceKind() == Action::OFK_HIP &&
+!C.getArgs().hasFlag(options::OPT_fgpu_rdc,
+ options::OPT_fno_gpu_rdc, false);
+  if (IsHIPNoRDC) {
+Output = BaseName;
+llvm::sys::path::replace_extension(Output, "");
+  }
   Output += OffloadingPrefix;
   if (MultipleArchs && !BoundArch.empty()) {
 Output += "-";
 Output.append(BoundArch);
   }
+  if (IsHIPNoRDC)
+Output += ".out";
   NamedOutput = C.getArgs().MakeArgString(Output.c_str());
 }
   } else if (JA.getType() == types::TY_PCH && IsCLMode()) {

Modified: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp?rev=374198&r1=374197&r2=374198&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp Wed Oct  9 11:46:43 2019
@@ -1387,14 +1387,12 @@ void tools::AddHIPLinkerScript(const Too
 
   // Create temporary linker script. Keep it if save-temps is enabled.
   const char *LKS;
-  SmallString<256> Name = llvm::sys::path::filename(Output.getFilename());
+  std::string Name = llvm::sys::path::filename(Output.getFilename());
   if (C.getDriver().isSaveTempsEnabled()) {
-llvm::sys::path::replace_extension(Name, "lk");
-LKS = C.getArgs().MakeArgString(Name.c_str());
+LKS = C.getArgs().MakeArgString(Name + ".lk");
   } else {
-llvm::sys::path::replace_extension(Name, "");
-Name = C.getDriver().GetTemporaryPath(Name, "lk");
-LKS = C.addTempFile(C.getArgs().MakeArgString(Name.c_str()));
+auto TmpName = C.getDriver().GetTemporaryPath(Name, "lk");
+LKS = C.addTempFile(C.getArgs().MakeArgString(TmpName));
   }
 
   // Add linker script option to the command.
@@ -1412,11 +1410,13 @@ void tools::AddHIPLinkerScript(const Too
  "Wrong platform");
   (void)HIPTC;
 
-  // The output file name needs to persist through the compilation, therefore
-  // it needs to be created through MakeArgString.
-  std::string BundleFileName = C.getDriver().GetTemporaryPath("BUNDLE", 
"hipfb");
-  const char *BundleFile =
-  C.addTempFile(C.getArgs().MakeArgString(BundleFileName.c_str()));
+  const char *BundleFile;
+  if (C.getDriver().isSaveTempsEnabled()) {
+BundleFile = C.getArgs().MakeArgString(Name + ".hipfb");
+  } else {
+auto TmpName = C.getDriver().GetTemporaryPath(Name, "hipfb");
+BundleFile = C.addTempFile(C.getArgs().MakeArgString(TmpName));
+  }
   AMDGCN::constructHIPFatbinCommand(C, JA, BundleFile, DeviceInputs, Args, T);
 
   // Add commands to embed target binaries. We ensure that each section and
@@ -1428,14 +1428,14 @@ void tools::AddHIPLinkerScript(const Too
   LksStream << " *** Automatically generated by Clang ***\n";
   LksStream << "*/\n";
   LksStream << "TARGET(binary)\n";
-  LksStream << "INPUT(" << BundleFileName << ")\n";
+  LksStream << "INPUT(" << BundleFile << ")\n";
   LksStream << "SECTIONS\n";
   LksStream << "{\n";
   LksStream << "  .hip_fatbin :\n";
   LksStream << "  ALIGN(0x10)\n";
   LksStream << "  {\n";
   LksStream << "PROVIDE_HIDDEN(__hip_fatbin = .);\n";
-  LksStream << "" << BundleFileName << "\n";
+  LksStream << "" << BundleFile << "\n";
   LksStream << "  }\n";
   LksStream << "  /DISCARD/ :\n";
   LksStream << "  {\n";

Modified: cfe/trunk/lib/Driver/ToolChains/HIP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/HIP.cpp?rev=374198&r1=374197&r2=374198&view=diff

[PATCH] D68715: [mangle] Fix mangling where an extra mangle context is required.

[Michael Liao via Phabricator via cfe-commits]

hliao updated this revision to Diff 224115.
hliao added a comment.

remove refactoring code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68715

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/test/CodeGenCXX/mangle-lambdas.cpp

Index: clang/test/CodeGenCXX/mangle-lambdas.cpp
===
--- clang/test/CodeGenCXX/mangle-lambdas.cpp
+++ clang/test/CodeGenCXX/mangle-lambdas.cpp
@@ -178,18 +178,24 @@
 }
 
 namespace std {
-  struct type_info;
+  struct type_info {
+bool before(const type_info &) const noexcept;
+  };
 }
 namespace PR12123 {
   struct A { virtual ~A(); } g;
+  struct C { virtual ~C(); } k;
   struct B {
 void f(const std::type_info& x = typeid([]()->A& { return g; }()));
 void h();
+void j(bool cond = typeid([]() -> A & { return g; }()).before(typeid([]() -> C & { return k; }(;
   };
-  void B::h() { f(); }
+  void B::h() { f(); j(); }
 }
 
 // CHECK-LABEL: define linkonce_odr dereferenceable({{[0-9]+}}) %"struct.PR12123::A"* @_ZZN7PR121231B1fERKSt9type_infoEd_NKUlvE_clEv
+// CHECK-LABEL: define linkonce_odr dereferenceable({{[0-9]+}}) %"struct.PR12123::A"* @_ZZN7PR121231B1jEbEd_NKUlvE_clEv
+// CHECK-LABEL: define linkonce_odr dereferenceable({{[0-9]+}}) %"struct.PR12123::C"* @_ZZN7PR121231B1jEbEd_NKUlvE0_clEv
 
 // CHECK-LABEL: define {{.*}} @_Z{{[0-9]*}}testVarargsLambdaNumberingv(
 inline int testVarargsLambdaNumbering() {
Index: clang/lib/Sema/SemaLambda.cpp
===
--- clang/lib/Sema/SemaLambda.cpp
+++ clang/lib/Sema/SemaLambda.cpp
@@ -352,21 +352,13 @@
 //  -- the initializers of inline variables
   case VariableTemplate:
 //  -- the initializers of templated variables
-return &ExprEvalContexts.back().getMangleNumberingContext(Context);
+return &Context.getManglingNumberContext(ASTContext::NeedExtraManglingDecl,
+ ManglingContextDecl);
   }
 
   llvm_unreachable("unexpected context");
 }
 
-MangleNumberingContext &
-Sema::ExpressionEvaluationContextRecord::getMangleNumberingContext(
-ASTContext &Ctx) {
-  assert(ManglingContextDecl && "Need to have a context declaration");
-  if (!MangleNumbering)
-MangleNumbering = Ctx.createMangleNumberingContext();
-  return *MangleNumbering;
-}
-
 CXXMethodDecl *Sema::startLambdaDefinition(
 CXXRecordDecl *Class, SourceRange IntroducerRange,
 TypeSourceInfo *MethodTypeInfo, SourceLocation EndLoc,
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -10261,6 +10261,16 @@
   return *MCtx;
 }
 
+MangleNumberingContext &
+ASTContext::getManglingNumberContext(NeedExtraManglingDecl_t, const Decl *D) {
+  assert(LangOpts.CPlusPlus); // We don't need mangling numbers for plain C.
+  std::unique_ptr &MCtx =
+  ExtraMangleNumberingContexts[D];
+  if (!MCtx)
+MCtx = createMangleNumberingContext();
+  return *MCtx;
+}
+
 std::unique_ptr
 ASTContext::createMangleNumberingContext() const {
   return ABI->createMangleNumberingContext();
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -1045,13 +1045,6 @@
 /// suffice, e.g., in a default function argument.
 Decl *ManglingContextDecl;
 
-/// The context information used to mangle lambda expressions
-/// and block literals within this context.
-///
-/// This mangling information is allocated lazily, since most contexts
-/// do not have lambda expressions or block literals.
-std::unique_ptr MangleNumbering;
-
 /// If we are processing a decltype type, a set of call expressions
 /// for which we have deferred checking the completeness of the return type.
 SmallVector DelayedDecltypeCalls;
@@ -1080,12 +1073,7 @@
   ExpressionKind ExprContext)
 : Context(Context), ParentCleanup(ParentCleanup),
   NumCleanupObjects(NumCleanupObjects), NumTypos(0),
-  ManglingContextDecl(ManglingContextDecl), MangleNumbering(),
-  ExprContext(ExprContext) {}
-
-/// Retrieve the mangling numbering context, used to consistently
-/// number constructs like lambdas for mangling.
-MangleNumberingContext &getMangleNumberingContext(ASTContext &Ctx);
+  ManglingContextDecl(ManglingContextDecl), ExprContext(ExprContext) {}
 
 bool isUnevaluated() const {
   return Context == ExpressionEvaluationContext::Unevaluated ||
Index: clang/include/clang/AST/ASTContext.h
===
--- clang/include/clang/AST/ASTCon