[clang] f1cd659 - [AST][FPEnv] Keep FP options in trailing storage of CastExpr

2020-09-13 Thread Serge Pavlov via cfe-commits 

Author: Serge Pavlov
Date: 2020-09-14T12:15:21+07:00
New Revision: f1cd6593da3ad763eb3f7aaf7761d06fb303493a

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

LOG: [AST][FPEnv] Keep FP options in trailing storage of CastExpr

This is recommit of 6c8041aa0f, reverted in de044f7562 because of some
fails. Original commit message is below.

This change allow a CastExpr to have optional FPOptionsOverride object,
stored in trailing storage. Of all cast nodes only ImplicitCastExpr,
CStyleCastExpr, CXXFunctionalCastExpr and CXXStaticCastExpr are allowed
to have FPOptions.

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

Added: 


Modified: 
clang/include/clang/AST/Expr.h
clang/include/clang/AST/ExprCXX.h
clang/include/clang/AST/ExprObjC.h
clang/include/clang/AST/Stmt.h
clang/include/clang/AST/TextNodeDumper.h
clang/include/clang/Basic/LangOptions.h
clang/lib/AST/ASTImporter.cpp
clang/lib/AST/Expr.cpp
clang/lib/AST/ExprCXX.cpp
clang/lib/AST/TextNodeDumper.cpp
clang/lib/Analysis/BodyFarm.cpp
clang/lib/CodeGen/CGBlocks.cpp
clang/lib/CodeGen/CGObjC.cpp
clang/lib/CodeGen/CGStmtOpenMP.cpp
clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp
clang/lib/Frontend/Rewrite/RewriteObjC.cpp
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaCast.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/lib/Sema/SemaExprObjC.cpp
clang/lib/Sema/SemaInit.cpp
clang/lib/Sema/SemaLambda.cpp
clang/lib/Sema/SemaObjCProperty.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/lib/Sema/SemaOverload.cpp
clang/lib/Sema/SemaStmt.cpp
clang/lib/Sema/SemaTemplate.cpp
clang/lib/Serialization/ASTReaderStmt.cpp
clang/lib/Serialization/ASTWriterDecl.cpp
clang/lib/Serialization/ASTWriterStmt.cpp
clang/test/AST/ast-dump-fpfeatures.cpp

Removed: 




diff  --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index 26e52ad367f8..1672fd707c6d 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -3440,9 +3440,11 @@ class CastExpr : public Expr {
   }
   CXXBaseSpecifier **path_buffer();
 
+  friend class ASTStmtReader;
+
 protected:
   CastExpr(StmtClass SC, QualType ty, ExprValueKind VK, const CastKind kind,
-   Expr *op, unsigned BasePathSize)
+   Expr *op, unsigned BasePathSize, bool HasFPFeatures)
   : Expr(SC, ty, VK, OK_Ordinary), Op(op) {
 CastExprBits.Kind = kind;
 CastExprBits.PartOfExplicitCast = false;
@@ -3451,17 +3453,27 @@ class CastExpr : public Expr {
"BasePathSize overflow!");
 setDependence(computeDependence(this));
 assert(CastConsistency());
+CastExprBits.HasFPFeatures = HasFPFeatures;
   }
 
   /// Construct an empty cast.
-  CastExpr(StmtClass SC, EmptyShell Empty, unsigned BasePathSize)
-: Expr(SC, Empty) {
+  CastExpr(StmtClass SC, EmptyShell Empty, unsigned BasePathSize,
+   bool HasFPFeatures)
+  : Expr(SC, Empty) {
 CastExprBits.PartOfExplicitCast = false;
 CastExprBits.BasePathSize = BasePathSize;
+CastExprBits.HasFPFeatures = HasFPFeatures;
 assert((CastExprBits.BasePathSize == BasePathSize) &&
"BasePathSize overflow!");
   }
 
+  /// Return a pointer to the trailing FPOptions.
+  /// \pre hasStoredFPFeatures() == true
+  FPOptionsOverride *getTrailingFPFeatures();
+  const FPOptionsOverride *getTrailingFPFeatures() const {
+return const_cast(this)->getTrailingFPFeatures();
+  }
+
 public:
   CastKind getCastKind() const { return (CastKind) CastExprBits.Kind; }
   void setCastKind(CastKind K) { CastExprBits.Kind = K; }
@@ -3506,6 +3518,28 @@ class CastExpr : public Expr {
 return getTargetFieldForToUnionCast(getType(), getSubExpr()->getType());
   }
 
+  bool hasStoredFPFeatures() const { return CastExprBits.HasFPFeatures; }
+
+  /// Get FPOptionsOverride from trailing storage.
+  FPOptionsOverride getStoredFPFeatures() const {
+assert(hasStoredFPFeatures());
+return *getTrailingFPFeatures();
+  }
+
+  // Get the FP features status of this operation. Only meaningful for
+  // operations on floating point types.
+  FPOptions getFPFeaturesInEffect(const LangOptions ) const {
+if (hasStoredFPFeatures())
+  return getStoredFPFeatures().applyOverrides(LO);
+return FPOptions::defaultWithoutTrailingStorage(LO);
+  }
+
+  FPOptionsOverride getFPFeatures() const {
+if (hasStoredFPFeatures())
+  return getStoredFPFeatures();
+return FPOptionsOverride();
+  }
+
   static const FieldDecl *getTargetFieldForToUnionCast(QualType unionType,
QualType opType);
   static const FieldDecl

[PATCH] D87163: [DSE] Switch to MemorySSA-backed DSE by default.

2020-09-13 Thread Nico Weber via Phabricator via cfe-commits 
thakis added a comment.

Heads-up: We're seeing fairly widespread test failures with this in Chromium 
that go away when we force this flag off. It's possible that it's due to only a 
small number of issues and they might be previously-benign UB (no idea), but I 
figured I'd give an early note. We'll send an update once we've reduced a 
failing test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87163

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

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

2020-09-13 Thread Alexandre Ganea via Phabricator via cfe-commits 
aganea added a comment.

Thanks for working on this @russell.gallop!

I've reproduced your tests, please see below. The only difference is that I've 
used a ThinLTO build for stage2:
-DCMAKE_CXX_FLAGS="/GS- -Xclang -O3 -fstrict-aliasing -march=skylake-avx512 
-flto=thin -fwhole-program-vtables"
Running with `/opt:lldltojobs=all` no `/lldltocache`.
Results on a 36-core (dual mount) Xeon Gold 6140.

  (WinHeap vs. Scudo+options)
  
  D:\llvm-project>hyperfine -m 3 -w 1 "cd d:\llvm-project\buildninjaRelWinHeap3 
&& d:\llvm-project\buildninjaRelWinHeap2\bin\lld-link.exe 
@CMakeFiles\clang.rsp" "cd d:\llvm-project\buildninjaRelScudo3 && 
d:\llvm-project\buildninjaRelScudo2\bin\lld-link.exe @CMakeFiles\clang.rsp"
  Benchmark #1: cd d:\llvm-project\buildninjaRelWinHeap3 && 
d:\llvm-project\buildninjaRelWinHeap2\bin\lld-link.exe @CMakeFiles\clang.rsp
Time (mean ± σ): 664.086 s ± 18.740 s[User: 0.0 ms, System: 0.0 ms] 
 5
Range (min … max):   647.070 s … 684.172 s3 runs
  
  Benchmark #2: cd d:\llvm-project\buildninjaRelScudo3 && 
d:\llvm-project\buildninjaRelScudo2\bin\lld-link.exe @CMakeFiles\clang.rsp
Time (mean ± σ): 145.619 s ±  0.140 s[User: 0.0 ms, System: 8.1 ms] 
 0
Range (min … max):   145.522 s … 145.779 s3 runs
  
  Summary
'cd d:\llvm-project\buildninjaRelScudo3 && 
d:\llvm-project\buildninjaRelScudo2\bin\lld-link.exe @CMakeFiles\clang.rsp' ran
  4.56 ± 0.13 times faster than 'cd d:\llvm-project\buildninjaRelWinHeap3 
&& d:\llvm-project\buildninjaRelWinHeap2\bin\lld-link.exe @CMakeFiles\clang.rsp'

  (Scudo+options vs. Rpmalloc)
  
  D:\llvm-project>hyperfine -m 3 -w 1 "cd 
d:\llvm-project\buildninjaRelRpmalloc3 && 
d:\llvm-project\buildninjaRelRpMalloc2\bin\lld-link.exe @CMakeFiles\clang.rsp" 
"cd d:\llvm-project\buildninjaRelScudo3 && 
d:\llvm-project\buildninjaRelScudo2\bin\lld-link.exe @CMakeFiles\clang.rsp"
  Benchmark #1: cd d:\llvm-project\buildninjaRelRpmalloc3 && 
d:\llvm-project\buildninjaRelRpMalloc2\bin\lld-link.exe @CMakeFiles\clang.rsp
Time (mean ± σ): 95.423 s ±  0.830 s[User: 0.0 ms, System: 9.0 ms]  
 0
Range (min … max):   94.886 s … 96.380 s3 runs
  
  Benchmark #2: cd d:\llvm-project\buildninjaRelScudo3 && 
d:\llvm-project\buildninjaRelScudo2\bin\lld-link.exe @CMakeFiles\clang.rsp
Time (mean ± σ): 145.266 s ±  0.387 s[User: 4.9 ms, System: 7.6 ms] 
 6
Range (min … max):   144.894 s … 145.666 s3 runs
  
  Summary
'cd d:\llvm-project\buildninjaRelRpmalloc3 && 
d:\llvm-project\buildninjaRelRpMalloc2\bin\lld-link.exe @CMakeFiles\clang.rsp' 
ran
  1.52 ± 0.01 times faster than 'cd d:\llvm-project\buildninjaRelScudo3 && 
d:\llvm-project\buildninjaRelScudo2\bin\lld-link.exe @CMakeFiles\clang.rsp'

  (Scudo vs. Rpmalloc)
  
  D:\llvm-project>hyperfine -m 3 -w 1 "cd 
d:\llvm-project\buildninjaRelRpmalloc3 && 
d:\llvm-project\buildninjaRelRpMalloc2\bin\lld-link.exe @CMakeFiles\clang.rsp" 
"cd d:\llvm-project\buildninjaRelScudo3 && 
d:\llvm-project\buildninjaRelScudo2\bin\lld-link.exe @CMakeFiles\clang.rsp"
  Benchmark #1: cd d:\llvm-project\buildninjaRelRpmalloc3 && 
d:\llvm-project\buildninjaRelRpMalloc2\bin\lld-link.exe @CMakeFiles\clang.rsp
Time (mean ± σ): 95.435 s ±  0.059 s[User: 0.0 ms, System: 8.0 ms]  
 0
Range (min … max):   95.385 s … 95.499 s3 runs
  
  Benchmark #2: cd d:\llvm-project\buildninjaRelScudo3 && 
d:\llvm-project\buildninjaRelScudo2\bin\lld-link.exe @CMakeFiles\clang.rsp
Time (mean ± σ): 270.967 s ±  1.366 s[User: 4.8 ms, System: 0.0 ms] 
 0
Range (min … max):   269.397 s … 271.887 s3 runs
  
  Summary
'cd d:\llvm-project\buildninjaRelRpmalloc3 && 
d:\llvm-project\buildninjaRelRpMalloc2\bin\lld-link.exe @CMakeFiles\clang.rsp' 
ran
  2.84 ± 0.01 times faster than 'cd d:\llvm-project\buildninjaRelScudo3 && 
d:\llvm-project\buildninjaRelScudo2\bin\lld-link.exe @CMakeFiles\clang.rsp'

Summary:

|   | Time | Factor |
| WinHeap   | 664.086 s ± 18.740 s | 1.0|
| Scudo | 270.967 s ±  1.366 s | 2.45   |
| Scudo+options | 145.619 s ±  0.140 s | 4.56   |
| Rpmalloc  | 95.423 s ±  0.830 s  | 6.95   |
|

CPU usage:
Rpmaloc - 3,944 cumulated seconds (all threads)
F12940831: image.png 
Scudo+options - 6,337 cumulated seconds (all threads)
F12940833: image.png 

Time spent in the allocator itself (note the different vertical scale in the 
graph)
(a hardware CRC or AES implemention will certainly help for Scudo)
Rpmalloc - 191 cumulated seconds
F12940851: image.png 
Scudo+options - 1,171 cumulated seconds
F12940855: image.png

[PATCH] D86048: [AST][RecoveryExpr] Popagate the error-bit from a VarDecl's initializer to DeclRefExpr.

2020-09-13 Thread Richard Smith - zygoloid via Phabricator via cfe-commits 
rsmith added a comment.

In D86048#2254607 , @sammccall wrote:

> The crux is we're forcing `decltype(N)` to be a (unique) dependent type, 
> which feels wrong.
> [...]
> There are three behaviors:
>
> - standard up to C++14: traversing the expr looking for a template param to 
> be lexically included (this is my reading of the standard)

FWIW, "involves a template parameter" is exactly the same phrasing that 
[temp.over.link] uses to refer to instantiation-dependence; that's why Clang 
uses instantiation-dependence in this case at the moment.

> - what clang actually does: check instantiation dependence, which I think 
> pulls in too many cases
> - standard after http://wg21.link/cwg2064: check type dependence
>
> I think it's probably OK to adopt the C++17 behavior for all versions (if I'm 
> right that the current behavior is a bug).
> @rsmith It's your DR, what do you think :-)

Let's try it and see what happens. I think this will reveal a collection of 
cases where we don't properly handle 
instantiation-dependent-but-not-type-dependent constructs (such as, if I 
remember correctly, the types of non-type template parameters), but we should 
be fixing those bugs anyway :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86048

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

[PATCH] D87590: Backport D79219, D85820, D86134 to 10.0 branch

2020-09-13 Thread Brian J. Cardiff via Phabricator via cfe-commits 
bcardiff created this revision.
bcardiff added reviewers: JDevlieghere, phosek, smeenai.
Herald added subscribers: llvm-commits, lldb-commits, Sanitizers, cfe-commits, 
hiraditya, mgorny.
Herald added projects: clang, Sanitizers, LLDB, LLVM.
bcardiff requested review of this revision.

Changes CMake configuration files are backported from master to 10.0 branch.

The 10.0.1 release is a bit broken and affects homebrew 
https://discourse.brew.sh/t/llvm-config-10-0-1-advertise-libxml2-tbd-as-system-libs/8593

To backport https://reviews.llvm.org/D79219 I needed to go through many commits 
of changes and revert changes regarding "Simplify CMake handling for zlib".

The other changes included are:

- https://reviews.llvm.org/D85820 "Use find_library for ncurses" and,
- https://reviews.llvm.org/D86134 "Fix OCaml build failure because of absolute 
path in system libs".

I'm not sure if there are plans for another 10.x release, but since I went 
through the history to make the patch for homebrew I figure I would send it 
here also just in case.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87590

Files:
  clang/CMakeLists.txt
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/cmake/config-ix.cmake
  compiler-rt/lib/xray/tests/CMakeLists.txt
  compiler-rt/test/lit.common.configured.in
  lld/CMakeLists.txt
  lld/test/CMakeLists.txt
  lld/test/lit.site.cfg.py.in
  lldb/cmake/modules/LLDBStandalone.cmake
  lldb/source/Core/CMakeLists.txt
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  llvm/cmake/config-ix.cmake
  llvm/cmake/modules/LLVMConfig.cmake.in
  llvm/include/llvm/Config/config.h.cmake
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CRC.cpp
  llvm/lib/Support/Compression.cpp
  llvm/lib/Support/Unix/Process.inc
  llvm/test/CMakeLists.txt
  llvm/test/lit.site.cfg.py.in
  llvm/unittests/Support/CompressionTest.cpp
  llvm/utils/gn/secondary/clang/test/BUILD.gn
  llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
  llvm/utils/gn/secondary/lld/test/BUILD.gn
  llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
  llvm/utils/gn/secondary/llvm/test/BUILD.gn

Index: llvm/utils/gn/secondary/llvm/test/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/test/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/test/BUILD.gn
@@ -166,9 +166,9 @@
   }
 
   if (llvm_enable_zlib) {
-extra_values += [ "HAVE_LIBZ=1" ]
+extra_values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-extra_values += [ "HAVE_LIBZ=0" ]  # Must be 0.
+extra_values += [ "LLVM_ENABLE_ZLIB=0" ]  # Must be 0.
   }
 }
 
Index: llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
@@ -294,9 +294,9 @@
   }
 
   if (llvm_enable_terminfo) {
-values += [ "HAVE_TERMINFO=1" ]
+values += [ "LLVM_ENABLE_TERMINFO=1" ]
   } else {
-values += [ "HAVE_TERMINFO=" ]
+values += [ "LLVM_ENABLE_TERMINFO=" ]
   }
 
   if (llvm_enable_dia_sdk) {
Index: llvm/utils/gn/secondary/lld/test/BUILD.gn
===
--- llvm/utils/gn/secondary/lld/test/BUILD.gn
+++ llvm/utils/gn/secondary/lld/test/BUILD.gn
@@ -49,9 +49,9 @@
   }
 
   if (llvm_enable_zlib) {
-extra_values += [ "HAVE_LIBZ=1" ]
+extra_values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-extra_values += [ "HAVE_LIBZ=0" ]  # Must be 0.
+extra_values += [ "LLVM_ENABLE_ZLIB=0" ]  # Must be 0.
   }
 
   if (current_cpu == "x64" || current_cpu == "arm64" ||
Index: llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
===
--- llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
+++ llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
@@ -85,8 +85,8 @@
   }
 
   if (llvm_enable_zlib) {
-values += [ "HAVE_LIBZ=1" ]
+values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-values += [ "HAVE_LIBZ=0" ]
+values += [ "LLVM_ENABLE_ZLIB=0" ]
   }
 }
Index: llvm/utils/gn/secondary/clang/test/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/test/BUILD.gn
+++ llvm/utils/gn/secondary/clang/test/BUILD.gn
@@ -74,9 +74,9 @@
   }
 
   if (llvm_enable_zlib) {
-extra_values += [ "HAVE_LIBZ=1" ]
+extra_values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-extra_values += [ "HAVE_LIBZ=0" ]  # Must be 0.
+extra_values += [ "LLVM_ENABLE_ZLIB=0" ]  # Must be 0.
   }
 
   if (host_cpu == "x64") {
Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -21,7 +21,7 @@
 
 namespace {
 
-#if

[PATCH] D87565: [Sema] Improve const_cast conformance to N4261

2020-09-13 Thread Richard Smith - zygoloid via Phabricator via cfe-commits 
rsmith added a comment.

For what it's worth, the most recent thoughts from CWG on this question are 
from 2015 (prior to guaranteed copy elision, which probably changes the 
answer): http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#1965. That 
direction would permit `const_cast` to rvalue reference type to materialize a 
temporary for arrays and classes but not scalar types (which seems just as 
arbitrary as the current set of rules to me).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87565

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

[PATCH] D87565: [Sema] Improve const_cast conformance to N4261

2020-09-13 Thread Richard Smith - zygoloid via Phabricator via cfe-commits 
rsmith requested changes to this revision.
rsmith added inline comments.
This revision now requires changes to proceed.



Comment at: clang/lib/Sema/SemaCast.cpp:1781-1782
 //   type T2 using the cast const_cast; and
 //-- if T1 is a class type, a prvalue of type T1 can be explicitly
 //   converted to an xvalue of type T2 using the cast const_cast.
 

This is the bullet that causes class pvalues and non-class prvalues to be 
treated differently by `const_cast` to an rvalue reference type, and it still 
exists unchanged in the latest C++ draft. As far as I can tell, the change here 
is not in line with the wording for N4261 / CWG issue 330, which Clang does 
implement.

(Now, I think the change you're suggesting here makes a lot of sense, but we 
should get agreement in WG21 to fix the language rules first.)



Comment at: clang/lib/Sema/SemaCast.cpp:1797-1801
+  // C++17 [expr.const.cast]p3
+  // For two similar types T1 and T2, a prvalue of type T1 may be 
explicitly
+  // converted to the type T2 using a const_cast if, considering the
+  // cv-decompositions of both types, each P1i is the same as P2i for all 
i.
+  // The result of a const_cast refers to the original entity.

In (for example) `const_cast(0);`, `T1` is `int`, and `T2` is "rvalue 
reference to `int`". `T1` and `T2` are not similar types, because they have 
different types `U` (for `T1`, `U` is `int`; for `T2`, `U` is "rvalue reference 
to `int`"). So this doesn't apply.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87565

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

[PATCH] D87587: [clang-format] Make one-line namespaces resistant to FixNamespaceComments, update documentation

2020-09-13 Thread Jake Merdich via Phabricator via cfe-commits 
JakeMerdichAMD edited reviewers, added: MyDeveloperDay; removed: jmerdich.
JakeMerdichAMD requested changes to this revision.
JakeMerdichAMD added a subscriber: MyDeveloperDay.
JakeMerdichAMD added a comment.
This revision now requires changes to proceed.

After looking at more of the history (ie, the commit you referenced), I'd 
definitely be open to something like this, provided that it doesn't affect 
namespaces that reside completely on one line. Since it was mostly a 
clang-format limitation and relatively rare, I think we can change the default 
here, but that's not up to just me (+@MyDeveloperDay), and extra scrutiny is 
definitely required when changing existing tests.

In any case, can you also add the full diff context 
?
 It makes it easier for us to review.




Comment at: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp:426-428
+  EXPECT_EQ("namespace A { a }// namespace A",
+fixNamespaceEndComments("namespace A { a }"));
+  EXPECT_EQ("namespace A { a };// namespace A",

I strongly believe that these ones are not correct. Namespaces that are 
entirely on one line should never have a trailing comment, even if it has 
content in it. 

A solution would also have to take into account whether future passes would 
split this onto separate lines (and thus have a different result after 
re-running clang-format), which was the reason for this limitation in the first 
place. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87587

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

[PATCH] D87566: [Sema] Permit conversions to arrays of unknown bound

2020-09-13 Thread Richard Smith - zygoloid via Phabricator via cfe-commits 
rsmith added a comment.

I made a start on this a while back and found a bunch more places that needed 
updates: 
https://github.com/zygoloid/llvm-project/commit/c23440979ac4f07ce38657ce0e69d010d2afa83e

Feel free to either take that patch as a basis or as inspiration for the 
remaining changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87566

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

[PATCH] D87561: [Sema] List conversion validate character array

2020-09-13 Thread Richard Smith - zygoloid via Phabricator via cfe-commits 
rsmith added inline comments.



Comment at: clang/lib/Sema/SemaOverload.cpp:4988
+
+if (ToType->isArrayType() && ToType->isCharType() &&
+isa(From->getInit(0))) {

`isCharType` is too narrow a check here. We also need to check for all the 
other types that can be initialized from a string literal (`wchar_t`, 
`char16_t`, ... -- including `unsigned short` in some cases). These details are 
handled by 
[`IsStringInit`](https://github.com/llvm/llvm-project/blob/master/clang/lib/Sema/SemaInit.cpp#L136).
 Maybe it's worth exposing that as a `bool Sema::isStringInit` function or 
similar to use from here?



Comment at: clang/lib/Sema/SemaOverload.cpp:4989
+if (ToType->isArrayType() && ToType->isCharType() &&
+isa(From->getInit(0))) {
   InitializedEntity Entity =

This is too narrow a check in two ways: we should allow parenthesized string 
literals here, and we should allow `ObjCEncodeExpr`.



Comment at: clang/lib/Sema/SemaOverload.cpp:4991-4992
   InitializedEntity Entity =
-InitializedEntity::InitializeParameter(S.Context, ToType,
-   /*Consumed=*/false);
+  InitializedEntity::InitializeParameter(S.Context, ToType,
+ /*Consumed=*/false);
   if (S.CanPerformCopyInitialization(Entity, From)) {

Phabricator thinks you converted spaces to tabs here. Please can you check and 
convert back if necessary?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87561

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

[PATCH] D86841: [clang] Add mayprogress and llvm.loop.mustprogress attribute deduction

2020-09-13 Thread Atmn Patel via Phabricator via cfe-commits 
atmnpatel updated this revision to Diff 291471.
atmnpatel added a comment.

Flipped direction.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86841

Files:
  clang/lib/CodeGen/CGLoopInfo.cpp
  clang/lib/CodeGen/CGLoopInfo.h
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/address-safety-attr-flavors.cpp
  clang/test/CodeGen/address-safety-attr.cpp
  clang/test/CodeGen/attr-noprogress.c
  clang/test/CodeGen/attr-noprogress.cpp
  clang/test/CodeGen/memtag-attr.cpp
  clang/test/CodeGen/no-builtin.cpp
  clang/test/CodeGenCXX/cxx11-trivial-initializer-struct.cpp
  clang/test/CodeGenCXX/fno-unroll-loops-metadata.cpp
  clang/test/CodeGenCXX/thunks-ehspec.cpp
  clang/test/CodeGenCXX/thunks.cpp
  clang/test/OpenMP/simd_metadata.c
  clang/test/Profile/c-unprofiled-blocks.c
  clang/test/utils/update_cc_test_checks/Inputs/basic-cplusplus.cpp.expected
  
clang/test/utils/update_cc_test_checks/Inputs/check-attributes.cpp.funcattrs.expected

Index: clang/test/utils/update_cc_test_checks/Inputs/check-attributes.cpp.funcattrs.expected
===
--- clang/test/utils/update_cc_test_checks/Inputs/check-attributes.cpp.funcattrs.expected
+++ clang/test/utils/update_cc_test_checks/Inputs/check-attributes.cpp.funcattrs.expected
@@ -11,7 +11,7 @@
   struct RT Z;
 };
 
-// CHECK: Function Attrs: noinline nounwind optnone
+// CHECK: Function Attrs: noinline nounwind optnone mustprogress
 // CHECK-LABEL: @_Z3fooP2ST(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:[[S_ADDR:%.*]] = alloca %struct.ST*, align 8
Index: clang/test/utils/update_cc_test_checks/Inputs/basic-cplusplus.cpp.expected
===
--- clang/test/utils/update_cc_test_checks/Inputs/basic-cplusplus.cpp.expected
+++ clang/test/utils/update_cc_test_checks/Inputs/basic-cplusplus.cpp.expected
@@ -44,7 +44,7 @@
 // CHECK-NEXT:[[THIS_ADDR:%.*]] = alloca %class.Foo*, align 8
 // CHECK-NEXT:store %class.Foo* [[THIS:%.*]], %class.Foo** [[THIS_ADDR]], align 8
 // CHECK-NEXT:[[THIS1:%.*]] = load %class.Foo*, %class.Foo** [[THIS_ADDR]], align 8
-// CHECK-NEXT:call void @_ZN3FooD2Ev(%class.Foo* [[THIS1]]) [[ATTR2:#.*]]
+// CHECK-NEXT:call void @_ZN3FooD2Ev(%class.Foo* [[THIS1]]) [[ATTR3:#.*]]
 // CHECK-NEXT:ret void
 //
 Foo::~Foo() {}
@@ -70,7 +70,7 @@
 // CHECK-NEXT:call void @_ZN3FooC1Ei(%class.Foo* [[F]], i32 1)
 // CHECK-NEXT:[[CALL:%.*]] = call i32 @_ZNK3Foo23function_defined_inlineEi(%class.Foo* [[F]], i32 2)
 // CHECK-NEXT:[[CALL1:%.*]] = call i32 @_ZNK3Foo28function_defined_out_of_lineEi(%class.Foo* [[F]], i32 3)
-// CHECK-NEXT:call void @_ZN3FooD1Ev(%class.Foo* [[F]]) [[ATTR2]]
+// CHECK-NEXT:call void @_ZN3FooD1Ev(%class.Foo* [[F]]) [[ATTR3]]
 // CHECK-NEXT:ret i32 0
 //
 int main() {
Index: clang/test/Profile/c-unprofiled-blocks.c
===
--- clang/test/Profile/c-unprofiled-blocks.c
+++ clang/test/Profile/c-unprofiled-blocks.c
@@ -16,7 +16,7 @@
   // PGOUSE: br i1 %{{[^,]*}}, label %{{[^,]*}}, label %{{[^,]*}}{{$}}
   while (--i) {}
 
-  // PGOUSE: br i1 %{{[^,]*}}, label %{{[^,]*}}, label %{{[^,]*}}{{$}}
+  // PGOUSE: br i1 %{{[^,]*}}, label %{{[^,]*}}, label %{{[^,]*}}, !llvm.loop [[LOOP1:!.*]]
   do {} while (i++ < 75);
 
   // PGOUSE: switch {{.*}} [
@@ -46,7 +46,7 @@
 // PGOUSE: br i1 %{{[^,]*}}, label %{{[^,]*}}, label %{{[^,]*}}{{$}}
 while (--i) {}
 
-// PGOUSE: br i1 %{{[^,]*}}, label %{{[^,]*}}, label %{{[^,]*}}{{$}}
+// PGOUSE: br i1 %{{[^,]*}}, label %{{[^,]*}}, label %{{[^,]*}}, !llvm.loop [[LOOP2:!.*]]
 do {} while (i++ < 75);
 
 // PGOUSE: switch {{.*}} [
Index: clang/test/OpenMP/simd_metadata.c
===
--- clang/test/OpenMP/simd_metadata.c
+++ clang/test/OpenMP/simd_metadata.c
@@ -137,7 +137,8 @@
 }
 // CHECK: store float {{.+}}, float* {{.+}}, align {{.+}}, !llvm.access.group ![[ACCESS_GROUP_13:[0-9]+]]
   }
-// CHECK: br label %{{.+}}, !llvm.loop [[LOOP_H3_HEADER:![0-9]+]]
+  // CHECK: br label %{{.+}}, !llvm.loop [[LOOP_H3_HEADER_INNER:![0-9]+]]
+  // CHECK: br label %{{.+}}, !llvm.loop [[LOOP_H3_HEADER:![0-9]+]]
 }
 
 // Metadata for h1:
Index: clang/test/CodeGenCXX/thunks.cpp
===
--- clang/test/CodeGenCXX/thunks.cpp
+++ clang/test/CodeGenCXX/thunks.cpp
@@ -529,7 +529,7 @@
 // CHECK-NONOPT-LABEL: define linkonce_odr void @_ZThn8_N6Test101C3fooEv
 
 // Checking with opt
-// CHECK-OPT-LABEL: define internal void @_ZThn8_N6Test4B12_GLOBAL__N_11C1fEv(%"struct.Test4B::(anonymous namespace)::C"* %this) unnamed_addr #0 align 2
+// CHECK-OPT-LABEL: define internal void @_ZThn8_N6Test4B12_GLOBAL__N_11C1fEv(%"struct.Test4B::(anonymous namespace)::C"* %this)

[PATCH] D87563: [Sema] Improve overload resolution

2020-09-13 Thread Richard Smith - zygoloid via Phabricator via cfe-commits 
rsmith added a comment.

Looks like this is an implementation of DR1307. Please also add a test to 
`test/CXX/drs/dr13xx.cpp`.




Comment at: clang/include/clang/Sema/Overload.h:548-549
+
+/// The std::initializer_list expression to convert from.
+const InitListExpr *StandardInitializerListFrom{nullptr};
+

Storing the `IntListExpr` here doesn't seem like it captures the necessary 
information. Consider:

```
void f2(std::pair (&&)[]); // #1
void f2(std::string (&&)[]); // #2
void g2() { f2({"foo","bar"}); } // should call #2
```

Here, the `InitListExpr` in both cases has two elements. We don't store the 
converted `InitListExpr`, because we don't -- and aren't allowed to -- build 
the converted `InitListExpr` until we've chosen the right overload. But I think 
what we're supposed to do in this case is to notice that calling #1 would 
initialize only one element of the array (one pair), whereas calling #2 would 
initialize two elements of the array, so we should call #2 here.

In effect, what you need to do is to compute the effective array bound for the 
case where we're initializing an array of unknown bound, and prefer the 
conversion sequence with the lower effective array bound. `InitListChecker` 
already works this out internally, see 
[here](https://github.com/llvm/llvm-project/blob/c0bcd11068fc13e45b253c6c315882097f94c121/clang/lib/Sema/SemaInit.cpp#L1937),
 but currently only does that when actually doing the conversion (when not in 
`VerifyOnly` mode); you'd need to adjust that.

Given that the code that needs this (the P0388 part) is dead code for now, 
perhaps the best thing would be to do only the array bound comparison in this 
patch, and we can compute the proper array bound for said comparison in a 
future patch.



Comment at: clang/lib/Sema/SemaOverload.cpp:3690-3703
+class CompareListInitializationSequences {
+  // List-initialization sequence L1 is a better conversion sequence than
+  // list-initialization sequence L2 if:
+  // ...
+  // - C++17:
+  //   L1 converts to type "array of N1 T", L2 converts to type "array of N2 
T",
+  //   and N1 is smaller than N2.,

I think this would be simpler if structured a little differently:

* Rename `CompareListInitializationSequences::Conversion` to 
`ListInitializationSequence`.
* Make `CompareListInitializationSequences::Compare` a static member of 
`ListInitializationSequence`.
* Move the element type comparison into `...::Compare` and remove the 
`CompareListInitializationSequences` class, inlining the (now very simple) 
constructor into its only user.



Comment at: clang/lib/Sema/SemaOverload.cpp:3856-3861
 if (ICS1.isStdInitializerListElement() &&
 !ICS2.isStdInitializerListElement())
   return ImplicitConversionSequence::Better;
 if (!ICS1.isStdInitializerListElement() &&
 ICS2.isStdInitializerListElement())
   return ImplicitConversionSequence::Worse;

Should this be part of `CompareListInitializationSequences` too?



Comment at: clang/lib/Sema/SemaOverload.cpp:5150-5156
+} else if (ToType->isConstantArrayType()) {
+  // Has the initializer list exactly N elements or fewer than N elements?
+  uint64_t Size = S.getASTContext()
+  .getAsConstantArrayType(ToType)
+  ->getSize()
+  .getZExtValue();
+  if (From->getNumInits() > Size)

Simplify, and we may as well avoid assuming that the size fits in 64 bits since 
it's easy to do so here (even though there are lots of other places where we 
make that assumption).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87563

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

[PATCH] D87587: [clang-format] Make one-line namespaces resistant to FixNamespaceComments, update documentation

2020-09-13 Thread Krystian Kuzniarek via Phabricator via cfe-commits 
kuzkry updated this revision to Diff 291468.
kuzkry added a comment.

Formatted code after CI failure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87587

Files:
  clang-tools-extra/unittests/clang-include-fixer/IncludeFixerTest.cpp
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/NamespaceEndCommentsFixer.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp

Index: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -142,7 +142,7 @@
   EXPECT_EQ("namespace A {\n"
 "namespace B {\n"
 "int i;\n"
-"}\n"
+"}// namespace B\n"
 "}// namespace A",
 fixNamespaceEndComments("namespace A {\n"
 "namespace B {\n"
@@ -423,8 +423,9 @@
 TEST_F(NamespaceEndCommentsFixerTest, DoesNotAddEndCommentForShortNamespace) {
   EXPECT_EQ("namespace {}", fixNamespaceEndComments("namespace {}"));
   EXPECT_EQ("namespace A {}", fixNamespaceEndComments("namespace A {}"));
-  EXPECT_EQ("namespace A { a }", fixNamespaceEndComments("namespace A { a }"));
-  EXPECT_EQ("namespace A { a };",
+  EXPECT_EQ("namespace A { a }// namespace A",
+fixNamespaceEndComments("namespace A { a }"));
+  EXPECT_EQ("namespace A { a };// namespace A",
 fixNamespaceEndComments("namespace A { a };"));
 }
 
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -211,7 +211,7 @@
   EXPECT_EQ("namespace N {\n"
 "\n"
 "int i;\n"
-"}",
+"}  // namespace N",
 format("namespace N {\n"
"\n"
"inti;\n"
@@ -220,7 +220,7 @@
   EXPECT_EQ("/* something */ namespace N {\n"
 "\n"
 "int i;\n"
-"}",
+"}  // namespace N",
 format("/* something */ namespace N {\n"
"\n"
"inti;\n"
@@ -229,7 +229,7 @@
   EXPECT_EQ("inline namespace N {\n"
 "\n"
 "int i;\n"
-"}",
+"}  // namespace N",
 format("inline namespace N {\n"
"\n"
"inti;\n"
@@ -238,7 +238,7 @@
   EXPECT_EQ("/* something */ inline namespace N {\n"
 "\n"
 "int i;\n"
-"}",
+"}  // namespace N",
 format("/* something */ inline namespace N {\n"
"\n"
"inti;\n"
@@ -247,7 +247,7 @@
   EXPECT_EQ("export namespace N {\n"
 "\n"
 "int i;\n"
-"}",
+"}  // namespace N",
 format("export namespace N {\n"
"\n"
"inti;\n"
@@ -369,7 +369,7 @@
   EXPECT_EQ("namespace {\n"
 "int i;\n"
 "\n"
-"}",
+"} // namespace",
 format("namespace {\n"
"int i;\n"
"\n"
@@ -8887,7 +8887,7 @@
 format("void  f  (  )  {  if  ( a )  return  }"));
   EXPECT_EQ("namespace N {\n"
 "void f()\n"
-"}",
+"} // namespace N",
 format("namespace  N  {  void f()  }"));
   EXPECT_EQ("namespace N {\n"
 "void f() {}\n"
@@ -9839,7 +9839,7 @@
   verifyFormat("namespace Foo\n"
"{\n"
"void Bar();\n"
-   "};",
+   "}; // namespace Foo",
Style);
 }
 
@@ -9872,7 +9872,7 @@
Style);
   verifyFormat("namespace Foo {\n"
"void Bar();\n"
-   "};",
+   "}; // namespace Foo",
Style);
 
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
@@ -9913,7 +9913,7 @@
   verifyFormat("namespace Foo\n"
"{\n"
"void Bar();\n"
-   "};",
+   "}; // namespace Foo",
Style);
 }
 
Index: clang/lib/Format/NamespaceEndCommentsFixer.cpp
===
--- clang/lib/Format/NamespaceEndCommentsFixer.cpp
+++ clang/lib/Format/NamespaceEndCommentsFixer.cpp
@@ -24,7 +24,7 @@
 namespace {
 // The maximal number of unwrapped lines that a short namespace spans.
 // Short namespaces don't need an end comment.
-static const int kShortNamespaceMaxLines = 1;
+static const int kShortNamespaceMaxLines = 0;
 
 // Computes the name of a namespace given the namespace token.
 // Returns "" for anonymous namespace.
Index:

[PATCH] D87587: [clang-format] Make one-line namespaces resistant to FixNamespaceComments, update documentation

2020-09-13 Thread Krystian Kuzniarek via Phabricator via cfe-commits 
kuzkry updated this revision to Diff 291466.
kuzkry added a comment.

Adjusted tests IncludeFixerTest.cpp after CI failure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87587

Files:
  clang-tools-extra/unittests/clang-include-fixer/IncludeFixerTest.cpp
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/NamespaceEndCommentsFixer.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp

Index: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -142,7 +142,7 @@
   EXPECT_EQ("namespace A {\n"
 "namespace B {\n"
 "int i;\n"
-"}\n"
+"}// namespace B\n"
 "}// namespace A",
 fixNamespaceEndComments("namespace A {\n"
 "namespace B {\n"
@@ -423,8 +423,9 @@
 TEST_F(NamespaceEndCommentsFixerTest, DoesNotAddEndCommentForShortNamespace) {
   EXPECT_EQ("namespace {}", fixNamespaceEndComments("namespace {}"));
   EXPECT_EQ("namespace A {}", fixNamespaceEndComments("namespace A {}"));
-  EXPECT_EQ("namespace A { a }", fixNamespaceEndComments("namespace A { a }"));
-  EXPECT_EQ("namespace A { a };",
+  EXPECT_EQ("namespace A { a }// namespace A",
+fixNamespaceEndComments("namespace A { a }"));
+  EXPECT_EQ("namespace A { a };// namespace A",
 fixNamespaceEndComments("namespace A { a };"));
 }
 
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -211,7 +211,7 @@
   EXPECT_EQ("namespace N {\n"
 "\n"
 "int i;\n"
-"}",
+"}  // namespace N",
 format("namespace N {\n"
"\n"
"inti;\n"
@@ -220,7 +220,7 @@
   EXPECT_EQ("/* something */ namespace N {\n"
 "\n"
 "int i;\n"
-"}",
+"}  // namespace N",
 format("/* something */ namespace N {\n"
"\n"
"inti;\n"
@@ -229,7 +229,7 @@
   EXPECT_EQ("inline namespace N {\n"
 "\n"
 "int i;\n"
-"}",
+"}  // namespace N",
 format("inline namespace N {\n"
"\n"
"inti;\n"
@@ -238,7 +238,7 @@
   EXPECT_EQ("/* something */ inline namespace N {\n"
 "\n"
 "int i;\n"
-"}",
+"}  // namespace N",
 format("/* something */ inline namespace N {\n"
"\n"
"inti;\n"
@@ -247,7 +247,7 @@
   EXPECT_EQ("export namespace N {\n"
 "\n"
 "int i;\n"
-"}",
+"}  // namespace N",
 format("export namespace N {\n"
"\n"
"inti;\n"
@@ -369,7 +369,7 @@
   EXPECT_EQ("namespace {\n"
 "int i;\n"
 "\n"
-"}",
+"} // namespace",
 format("namespace {\n"
"int i;\n"
"\n"
@@ -8887,7 +8887,7 @@
 format("void  f  (  )  {  if  ( a )  return  }"));
   EXPECT_EQ("namespace N {\n"
 "void f()\n"
-"}",
+"} // namespace N",
 format("namespace  N  {  void f()  }"));
   EXPECT_EQ("namespace N {\n"
 "void f() {}\n"
@@ -9839,7 +9839,7 @@
   verifyFormat("namespace Foo\n"
"{\n"
"void Bar();\n"
-   "};",
+   "}; // namespace Foo",
Style);
 }
 
@@ -9872,7 +9872,7 @@
Style);
   verifyFormat("namespace Foo {\n"
"void Bar();\n"
-   "};",
+   "}; // namespace Foo",
Style);
 
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
@@ -9913,7 +9913,7 @@
   verifyFormat("namespace Foo\n"
"{\n"
"void Bar();\n"
-   "};",
+   "}; // namespace Foo",
Style);
 }
 
Index: clang/lib/Format/NamespaceEndCommentsFixer.cpp
===
--- clang/lib/Format/NamespaceEndCommentsFixer.cpp
+++ clang/lib/Format/NamespaceEndCommentsFixer.cpp
@@ -24,7 +24,7 @@
 namespace {
 // The maximal number of unwrapped lines that a short namespace spans.
 // Short namespaces don't need an end comment.
-static const int kShortNamespaceMaxLines = 1;
+static const int kShortNamespaceMaxLines = 0;
 
 // Computes the name of a namespace given the namespace token.
 // Returns "" for anonymous namespace.
Index:

[PATCH] D87588: [ASTMatchers] extract public matchers from const-analysis into own patch

2020-09-13 Thread Jonas Toth via Phabricator via cfe-commits 
JonasToth updated this revision to Diff 291467.
JonasToth added a comment.

- fix left over merge marker and one formatting problem


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87588

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -741,6 +741,164 @@
 std::make_unique>("v", 4)));
 }
 
+TEST(ForEachArgumentWithParamType, ReportsNoFalsePositives) {
+  StatementMatcher ArgumentY =
+  declRefExpr(to(varDecl(hasName("y".bind("arg");
+  TypeMatcher IntType = qualType(isInteger()).bind("type");
+  StatementMatcher CallExpr =
+  callExpr(forEachArgumentWithParamType(ArgumentY, IntType));
+
+  // IntParam does not match.
+  EXPECT_TRUE(notMatches("void f(int* i) { int* y; f(y); }", CallExpr));
+  // ArgumentY does not match.
+  EXPECT_TRUE(notMatches("void f(int i) { int x; f(x); }", CallExpr));
+}
+
+TEST(ForEachArgumentWithParamType, MatchesCXXMemberCallExpr) {
+  StatementMatcher ArgumentY =
+  declRefExpr(to(varDecl(hasName("y".bind("arg");
+  TypeMatcher IntType = qualType(isInteger()).bind("type");
+  StatementMatcher CallExpr =
+  callExpr(forEachArgumentWithParamType(ArgumentY, IntType));
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  "struct S {"
+  "  const S& operator[](int i) { return *this; }"
+  "};"
+  "void f(S S1) {"
+  "  int y = 1;"
+  "  S1[y];"
+  "}",
+  CallExpr, std::make_unique>("type", 1)));
+
+  StatementMatcher CallExpr2 =
+  callExpr(forEachArgumentWithParamType(ArgumentY, IntType));
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  "struct S {"
+  "  static void g(int i);"
+  "};"
+  "void f() {"
+  "  int y = 1;"
+  "  S::g(y);"
+  "}",
+  CallExpr2, std::make_unique>("type", 1)));
+}
+
+TEST(ForEachArgumentWithParamType, MatchesCallExpr) {
+  StatementMatcher ArgumentY =
+  declRefExpr(to(varDecl(hasName("y".bind("arg");
+  TypeMatcher IntType = qualType(isInteger()).bind("type");
+  StatementMatcher CallExpr =
+  callExpr(forEachArgumentWithParamType(ArgumentY, IntType));
+
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  "void f(int i) { int y; f(y); }", CallExpr,
+  std::make_unique>("type")));
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  "void f(int i) { int y; f(y); }", CallExpr,
+  std::make_unique>("arg")));
+
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  "void f(int i, int j) { int y; f(y, y); }", CallExpr,
+  std::make_unique>("type", 2)));
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  "void f(int i, int j) { int y; f(y, y); }", CallExpr,
+  std::make_unique>("arg", 2)));
+}
+
+TEST(ForEachArgumentWithParamType, MatchesConstructExpr) {
+  StatementMatcher ArgumentY =
+  declRefExpr(to(varDecl(hasName("y".bind("arg");
+  TypeMatcher IntType = qualType(isInteger()).bind("type");
+  StatementMatcher ConstructExpr =
+  cxxConstructExpr(forEachArgumentWithParamType(ArgumentY, IntType));
+
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  "struct C {"
+  "  C(int i) {}"
+  "};"
+  "int y = 0;"
+  "C Obj(y);",
+  ConstructExpr, std::make_unique>("type")));
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  "struct C {"
+  "  C(int i) {}"
+  "};"
+  "int y = 0;"
+  "C Obj(y);",
+  ConstructExpr, std::make_unique>("arg")));
+}
+
+TEST(ForEachArgumentWithParamType, HandlesKandRFunctions) {
+  StatementMatcher ArgumentY =
+  declRefExpr(to(varDecl(hasName("y".bind("arg");
+  TypeMatcher IntType = qualType(isInteger()).bind("type");
+  StatementMatcher CallExpr =
+  callExpr(forEachArgumentWithParamType(ArgumentY, IntType));
+
+  EXPECT_TRUE(matchesC("void f();\n"
+   "void call_it(void) { int x, y; f(x, y); }\n"
+   "void f(a, b) int a, b; {}\n"
+   "void call_it2(void) { int x, y; f(x, y); }",
+   CallExpr));
+}
+
+TEST(ForEachArgumentWithParamType, HandlesBoundNodesForNonMatches) {
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  "void g(int i, int j) {"
+  "  int a;"
+  "  int b;"
+  "  int c;"
+  "  g(a, 0);"
+  "  g(a, b);"
+  "  g(0, b);"
+  "}",
+  functionDecl(
+  forEachDescendant(varDecl().bind("v")),
+  forEachDescendant(callExpr(forEachArgumentWithParamType(
+  declRefExpr(to(decl(equalsBoundNode("v", qualType(),
+  std::make_unique>("v", 4)));
+}
+
+TEST(ForEachArgumentWithParamType, MatchesFunctionPtrCalls) {
+  StatementMatcher ArgumentY =
+

[PATCH] D87588: [ASTMatchers] extract public matchers from const-analysis into own patch

2020-09-13 Thread Jonas Toth via Phabricator via cfe-commits 
JonasToth created this revision.
JonasToth added a reviewer: aaron.ballman.
Herald added subscribers: cfe-commits, steven.zhang.
Herald added a project: clang.
JonasToth requested review of this revision.

The analysis for const-ness of local variables required a view generally useful
matchers that are extracted into its own patch.

They are decompositionDecl and forEachArgumentWithParamType, that works
for calls through function pointers as well.

This is a reupload of https://reviews.llvm.org/D72505, that already landed,
but had to be reverted due to a GCC crash on powerpc
(https://reviews.llvm.org/rG4c48ea68e491cb42f1b5d43ffba89f6a7f0dadc4)

Because this took a long time to adress, i decided to redo this patch and
have a clean workflow.
I try to coordinate with someone that has a PPC to apply this patch and
test for the crash. If everything is fine, I intend to just commit.
If the crash is still happening, i hope to at least find the cause.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87588

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -741,6 +741,164 @@
 std::make_unique>("v", 4)));
 }
 
+TEST(ForEachArgumentWithParamType, ReportsNoFalsePositives) {
+  StatementMatcher ArgumentY =
+  declRefExpr(to(varDecl(hasName("y".bind("arg");
+  TypeMatcher IntType = qualType(isInteger()).bind("type");
+  StatementMatcher CallExpr =
+  callExpr(forEachArgumentWithParamType(ArgumentY, IntType));
+
+  // IntParam does not match.
+  EXPECT_TRUE(notMatches("void f(int* i) { int* y; f(y); }", CallExpr));
+  // ArgumentY does not match.
+  EXPECT_TRUE(notMatches("void f(int i) { int x; f(x); }", CallExpr));
+}
+
+TEST(ForEachArgumentWithParamType, MatchesCXXMemberCallExpr) {
+  StatementMatcher ArgumentY =
+  declRefExpr(to(varDecl(hasName("y".bind("arg");
+  TypeMatcher IntType = qualType(isInteger()).bind("type");
+  StatementMatcher CallExpr =
+  callExpr(forEachArgumentWithParamType(ArgumentY, IntType));
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  "struct S {"
+  "  const S& operator[](int i) { return *this; }"
+  "};"
+  "void f(S S1) {"
+  "  int y = 1;"
+  "  S1[y];"
+  "}",
+  CallExpr, std::make_unique>("type", 1)));
+
+  StatementMatcher CallExpr2 =
+  callExpr(forEachArgumentWithParamType(ArgumentY, IntType));
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  "struct S {"
+  "  static void g(int i);"
+  "};"
+  "void f() {"
+  "  int y = 1;"
+  "  S::g(y);"
+  "}",
+  CallExpr2, std::make_unique>("type", 1)));
+}
+
+TEST(ForEachArgumentWithParamType, MatchesCallExpr) {
+  StatementMatcher ArgumentY =
+  declRefExpr(to(varDecl(hasName("y".bind("arg");
+  TypeMatcher IntType = qualType(isInteger()).bind("type");
+  StatementMatcher CallExpr =
+  callExpr(forEachArgumentWithParamType(ArgumentY, IntType));
+
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  "void f(int i) { int y; f(y); }", CallExpr,
+  std::make_unique>("type")));
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  "void f(int i) { int y; f(y); }", CallExpr,
+  std::make_unique>("arg")));
+
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  "void f(int i, int j) { int y; f(y, y); }", CallExpr,
+  std::make_unique>("type", 2)));
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  "void f(int i, int j) { int y; f(y, y); }", CallExpr,
+  std::make_unique>("arg", 2)));
+}
+
+TEST(ForEachArgumentWithParamType, MatchesConstructExpr) {
+  StatementMatcher ArgumentY =
+  declRefExpr(to(varDecl(hasName("y".bind("arg");
+  TypeMatcher IntType = qualType(isInteger()).bind("type");
+  StatementMatcher ConstructExpr =
+  cxxConstructExpr(forEachArgumentWithParamType(ArgumentY, IntType));
+
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  "struct C {"
+  "  C(int i) {}"
+  "};"
+  "int y = 0;"
+  "C Obj(y);",
+  ConstructExpr, std::make_unique>("type")));
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  "struct C {"
+  "  C(int i) {}"
+  "};"
+  "int y = 0;"
+  "C Obj(y);",
+  ConstructExpr, std::make_unique>("arg")));
+}
+
+TEST(ForEachArgumentWithParamType, HandlesKandRFunctions) {
+  StatementMatcher ArgumentY =
+  declRefExpr(to(varDecl(hasName("y".bind("arg");
+  TypeMatcher IntType = qualType(isInteger()).bind("type");
+  StatementMatcher CallExpr =
+  callExpr(forEachArgumentWithParamType(ArgumentY, IntType));
+
+  EXPECT_TRUE(matchesC("void f();\n"
+   "void call_it(void) { int x, y; f(x, y); }\n"
+   "void f(a, b) int

[clang] c0bcd11 - [ASTImporter] Add basic support for comparing Stmts and compare function bodies

2020-09-13 Thread Raphael Isemann via cfe-commits 

Author: Raphael Isemann
Date: 2020-09-13T18:25:04+02:00
New Revision: c0bcd11068fc13e45b253c6c315882097f94c121

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

LOG: [ASTImporter] Add basic support for comparing Stmts and compare function 
bodies

Right now the ASTImporter assumes for most Expr nodes that they are always equal
which leads to non-compatible declarations ending up being merged. This patch
adds the basic framework for comparing Stmts (and with that also Exprs) and
implements the custom checks for a few Stmt subclasses. I'll implement the
remaining subclasses in follow up patches (mostly because there are a lot of
subclasses and some of them require further changes like having GNU language in
the testing framework)

The motivation for this is that in LLDB we try to import libc++ source code and
some of the types we are importing there contain expressions (e.g. because they
use `enable_if`), so those declarations are currently merged even if they
are completely different (e.g. `enable_if ...` and `enable_if
...` are currently considered equal which is clearly not true).

Reviewed By: martong, balazske

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

Added: 


Modified: 
clang/include/clang/AST/ASTStructuralEquivalence.h
clang/lib/AST/ASTStructuralEquivalence.cpp
clang/unittests/AST/StructuralEquivalenceTest.cpp

Removed: 




diff  --git a/clang/include/clang/AST/ASTStructuralEquivalence.h 
b/clang/include/clang/AST/ASTStructuralEquivalence.h
index 36a42070fd28..c958a16aba21 100644
--- a/clang/include/clang/AST/ASTStructuralEquivalence.h
+++ b/clang/include/clang/AST/ASTStructuralEquivalence.h
@@ -97,6 +97,13 @@ struct StructuralEquivalenceContext {
   /// \c VisitedDecls members) and can cause faulty equivalent results.
   bool IsEquivalent(QualType T1, QualType T2);
 
+  /// Determine whether the two statements are structurally equivalent.
+  /// Implementation functions (all static functions in
+  /// ASTStructuralEquivalence.cpp) must never call this function because that
+  /// will wreak havoc the internal state (\c DeclsToCheck and
+  /// \c VisitedDecls members) and can cause faulty equivalent results.
+  bool IsEquivalent(Stmt *S1, Stmt *S2);
+
   /// Find the index of the given anonymous struct/union within its
   /// context.
   ///

diff  --git a/clang/lib/AST/ASTStructuralEquivalence.cpp 
b/clang/lib/AST/ASTStructuralEquivalence.cpp
index 8b5b2444f1e2..fafcfce269d7 100644
--- a/clang/lib/AST/ASTStructuralEquivalence.cpp
+++ b/clang/lib/AST/ASTStructuralEquivalence.cpp
@@ -68,7 +68,12 @@
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/ExprConcepts.h"
+#include "clang/AST/ExprObjC.h"
+#include "clang/AST/ExprOpenMP.h"
 #include "clang/AST/NestedNameSpecifier.h"
+#include "clang/AST/StmtObjC.h"
+#include "clang/AST/StmtOpenMP.h"
 #include "clang/AST/TemplateBase.h"
 #include "clang/AST/TemplateName.h"
 #include "clang/AST/Type.h"
@@ -149,32 +154,230 @@ static bool 
IsStructurallyEquivalent(StructuralEquivalenceContext ,
   return true;
 }
 
-/// Determine structural equivalence of two expressions.
-static bool IsStructurallyEquivalent(StructuralEquivalenceContext ,
- const Expr *E1, const Expr *E2) {
-  if (!E1 || !E2)
-return E1 == E2;
+namespace {
+/// Encapsulates Stmt comparison logic.
+class StmtComparer {
+  StructuralEquivalenceContext 
+
+  // IsStmtEquivalent overloads. Each overload compares a specific statement
+  // and only has to compare the data that is specific to the specific 
statement
+  // class. Should only be called from TraverseStmt.
+
+  bool IsStmtEquivalent(const AddrLabelExpr *E1, const AddrLabelExpr *E2) {
+return IsStructurallyEquivalent(Context, E1->getLabel(), E2->getLabel());
+  }
+
+  bool IsStmtEquivalent(const AtomicExpr *E1, const AtomicExpr *E2) {
+return E1->getOp() == E2->getOp();
+  }
+
+  bool IsStmtEquivalent(const BinaryOperator *E1, const BinaryOperator *E2) {
+return E1->getOpcode() == E2->getOpcode();
+  }
 
-  if (auto *DE1 = dyn_cast(E1)) {
-auto *DE2 = dyn_cast(E2);
-if (!DE2)
+  bool IsStmtEquivalent(const CallExpr *E1, const CallExpr *E2) {
+// FIXME: IsStructurallyEquivalent requires non-const Decls.
+Decl *Callee1 = const_cast(E1->getCalleeDecl());
+Decl *Callee2 = const_cast(E2->getCalleeDecl());
+
+// Compare whether both calls know their callee.
+if (static_cast(Callee1) != static_cast(Callee2))
   return false;
+
+// Both calls have no callee, so nothing to do.
+if (!static_cast(Callee1))
+  return true;
+
+assert(Callee2);
+return IsStructurallyEquivalent(Context, Callee1, Callee2);
+  }
+
+  bool

[PATCH] D87444: [ASTImporter] Add basic support for comparing Stmts and compare function bodies

2020-09-13 Thread Raphael Isemann via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc0bcd11068fc: [ASTImporter] Add basic support for comparing 
Stmts and compare function bodies (authored by teemperor).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87444

Files:
  clang/include/clang/AST/ASTStructuralEquivalence.h
  clang/lib/AST/ASTStructuralEquivalence.cpp
  clang/unittests/AST/StructuralEquivalenceTest.cpp

Index: clang/unittests/AST/StructuralEquivalenceTest.cpp
===
--- clang/unittests/AST/StructuralEquivalenceTest.cpp
+++ clang/unittests/AST/StructuralEquivalenceTest.cpp
@@ -19,14 +19,10 @@
   std::unique_ptr AST0, AST1;
   std::string Code0, Code1; // Buffers for SourceManager
 
-  // Get a pair of node pointers into the synthesized AST from the given code
-  // snippets. To determine the returned node, a separate matcher is specified
-  // for both snippets. The first matching node is returned.
-  template 
-  std::tuple
-  makeDecls(const std::string , const std::string ,
-TestLanguage Lang, const MatcherType ,
-const MatcherType ) {
+  // Parses the source code in the specified language and sets the ASTs of
+  // the current test instance to the parse result.
+  void makeASTUnits(const std::string , const std::string ,
+TestLanguage Lang) {
 this->Code0 = SrcCode0;
 this->Code1 = SrcCode1;
 std::vector Args = getCommandLineArgsForTesting(Lang);
@@ -35,6 +31,17 @@
 
 AST0 = tooling::buildASTFromCodeWithArgs(Code0, Args, InputFileName);
 AST1 = tooling::buildASTFromCodeWithArgs(Code1, Args, InputFileName);
+  }
+
+  // Get a pair of node pointers into the synthesized AST from the given code
+  // snippets. To determine the returned node, a separate matcher is specified
+  // for both snippets. The first matching node is returned.
+  template 
+  std::tuple
+  makeDecls(const std::string , const std::string ,
+TestLanguage Lang, const MatcherType ,
+const MatcherType ) {
+makeASTUnits(SrcCode0, SrcCode1, Lang);
 
 NodeType *D0 = FirstDeclMatcher().match(
 AST0->getASTContext().getTranslationUnitDecl(), Matcher0);
@@ -47,14 +54,7 @@
   std::tuple
   makeTuDecls(const std::string , const std::string ,
   TestLanguage Lang) {
-this->Code0 = SrcCode0;
-this->Code1 = SrcCode1;
-std::vector Args = getCommandLineArgsForTesting(Lang);
-
-const char *const InputFileName = "input.cc";
-
-AST0 = tooling::buildASTFromCodeWithArgs(Code0, Args, InputFileName);
-AST1 = tooling::buildASTFromCodeWithArgs(Code1, Args, InputFileName);
+makeASTUnits(SrcCode0, SrcCode1, Lang);
 
 return std::make_tuple(AST0->getASTContext().getTranslationUnitDecl(),
AST1->getASTContext().getTranslationUnitDecl());
@@ -80,6 +80,56 @@
 return makeDecls(SrcCode0, SrcCode1, Lang, Matcher);
   }
 
+  // Wraps a Stmt and the ASTContext that contains it.
+  struct StmtWithASTContext {
+Stmt *S;
+ASTContext *Context;
+explicit StmtWithASTContext(Stmt , ASTContext )
+: S(), Context() {}
+explicit StmtWithASTContext(FunctionDecl *FD)
+: S(FD->getBody()), Context(>getASTContext()) {}
+  };
+
+  // Get a pair of node pointers into the synthesized AST from the given code
+  // snippets. To determine the returned node, a separate matcher is specified
+  // for both snippets. The first matching node is returned.
+  template 
+  std::tuple
+  makeStmts(const std::string , const std::string ,
+TestLanguage Lang, const MatcherType ,
+const MatcherType ) {
+makeASTUnits(SrcCode0, SrcCode1, Lang);
+
+Stmt *S0 = FirstDeclMatcher().match(
+AST0->getASTContext().getTranslationUnitDecl(), Matcher0);
+Stmt *S1 = FirstDeclMatcher().match(
+AST1->getASTContext().getTranslationUnitDecl(), Matcher1);
+
+return std::make_tuple(StmtWithASTContext(*S0, AST0->getASTContext()),
+   StmtWithASTContext(*S1, AST1->getASTContext()));
+  }
+
+  // Get a pair of node pointers into the synthesized AST from the given code
+  // snippets. The same matcher is used for both snippets.
+  template 
+  std::tuple
+  makeStmts(const std::string , const std::string ,
+TestLanguage Lang, const MatcherType ) {
+return makeStmts(SrcCode0, SrcCode1, Lang, AMatcher, AMatcher);
+  }
+
+  // Convenience function for makeStmts that wraps the code inside a function
+  // body.
+  template 
+  std::tuple
+  makeWrappedStmts(const std::string , const std::string ,
+   TestLanguage Lang, const MatcherType ) {
+auto Wrap = [](const std::string ) {
+  return "void wrapped() {" + Src + ";}";
+};
+return makeStmts(Wrap(SrcCode0),

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

2020-09-13 Thread Douglas Chen via Phabricator via cfe-commits 
dougpuob added a comment.

In D86671#2259492 , @njames93 wrote:

> In D86671#2259443 , @dougpuob wrote:
>
>> In D86671#2259364 , @njames93 wrote:
>>
>>> Did you upload this incorrectly again, context is missing and seems to be a 
>>> relative diff from a previous version of this patch?
>>
>> Sorry for it, I think it's my bad. It is possible that I manually merged the 
>> last master(github) with changes then updated them both via web interface ...
>>
>> Can I fix it if switch back to the base (`14948a0`) then merge all my 
>> changes, then update the diff again via web interface? Or do you have any 
>> better suggestion?
>>
>> I am curious about how do you know this mistake? You got error messages with 
>> `arc patch D86671` ?
>
> The no context is easy to spot as phab says context not available. Its easy 
> to find knowing that there is no mention of hungarian notation in the trunk 
> version of IdentifierNamingCheck.cpp, yet there is mention of that in the 
> before diff.
>
> The way I do my patches is I create a branch from the current master. Then 
> all commits go into that branch. When its time to update the PR I can just do 
> a diff from  to .
> Though I do use arcanist for my patches
>
>   arc diff master
>
> arcanist will check to see if the current branch has tags for PR and 
> automatically update that PR. Otherwise it will create a new PR.
> If it goes to create a new PR instead of updating an existing one you can 
> pass update
>
>   arc diff master --update D86671

@njames93, thank you. I updated three updates with the way you told me, seems 
work fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86671

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

[PATCH] D87587: [clang-format] Make one-line namespaces resistant to FixNamespaceComments, update documentation

2020-09-13 Thread Krystian Kuzniarek via Phabricator via cfe-commits 
kuzkry added a comment.

This fixes https://bugs.llvm.org/show_bug.cgi?id=47290.

@Jake
I decided I would at least create this pull request so you can decide whether 
you want it or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87587

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

[PATCH] D87587: [clang-format] Make one-line namespaces resistant to FixNamespaceComments, update documentation

2020-09-13 Thread Krystian Kuzniarek via Phabricator via cfe-commits 
kuzkry created this revision.
kuzkry added reviewers: krasimir, JakeMerdichAMD, jmerdich, rsmith.
kuzkry added a project: clang-format.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
kuzkry requested review of this revision.

clang-format documentation states that having enabled
FixNamespaceComments one may expect below code:

  c++
  namespace a {
  foo();
  }

to be turned into:

  c++
  namespace a {
  foo();
  } // namespace a

In reality, no "// namespace a" was added. The problem was too high
value of kShortNamespaceMaxLines, which is used while deciding whether
a namespace is long enough to be formatted.

As with 9163fe2, it remains to ensure clang-format is idempotent.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87587

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

Index: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -142,7 +142,7 @@
   EXPECT_EQ("namespace A {\n"
 "namespace B {\n"
 "int i;\n"
-"}\n"
+"}// namespace B\n"
 "}// namespace A",
 fixNamespaceEndComments("namespace A {\n"
 "namespace B {\n"
@@ -423,8 +423,9 @@
 TEST_F(NamespaceEndCommentsFixerTest, DoesNotAddEndCommentForShortNamespace) {
   EXPECT_EQ("namespace {}", fixNamespaceEndComments("namespace {}"));
   EXPECT_EQ("namespace A {}", fixNamespaceEndComments("namespace A {}"));
-  EXPECT_EQ("namespace A { a }", fixNamespaceEndComments("namespace A { a }"));
-  EXPECT_EQ("namespace A { a };",
+  EXPECT_EQ("namespace A { a }// namespace A",
+fixNamespaceEndComments("namespace A { a }"));
+  EXPECT_EQ("namespace A { a };// namespace A",
 fixNamespaceEndComments("namespace A { a };"));
 }
 
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -211,7 +211,7 @@
   EXPECT_EQ("namespace N {\n"
 "\n"
 "int i;\n"
-"}",
+"}  // namespace N",
 format("namespace N {\n"
"\n"
"inti;\n"
@@ -220,7 +220,7 @@
   EXPECT_EQ("/* something */ namespace N {\n"
 "\n"
 "int i;\n"
-"}",
+"}  // namespace N",
 format("/* something */ namespace N {\n"
"\n"
"inti;\n"
@@ -229,7 +229,7 @@
   EXPECT_EQ("inline namespace N {\n"
 "\n"
 "int i;\n"
-"}",
+"}  // namespace N",
 format("inline namespace N {\n"
"\n"
"inti;\n"
@@ -238,7 +238,7 @@
   EXPECT_EQ("/* something */ inline namespace N {\n"
 "\n"
 "int i;\n"
-"}",
+"}  // namespace N",
 format("/* something */ inline namespace N {\n"
"\n"
"inti;\n"
@@ -247,7 +247,7 @@
   EXPECT_EQ("export namespace N {\n"
 "\n"
 "int i;\n"
-"}",
+"}  // namespace N",
 format("export namespace N {\n"
"\n"
"inti;\n"
@@ -369,7 +369,7 @@
   EXPECT_EQ("namespace {\n"
 "int i;\n"
 "\n"
-"}",
+"} // namespace",
 format("namespace {\n"
"int i;\n"
"\n"
@@ -8887,7 +8887,7 @@
 format("void  f  (  )  {  if  ( a )  return  }"));
   EXPECT_EQ("namespace N {\n"
 "void f()\n"
-"}",
+"} // namespace N",
 format("namespace  N  {  void f()  }"));
   EXPECT_EQ("namespace N {\n"
 "void f() {}\n"
@@ -9839,7 +9839,7 @@
   verifyFormat("namespace Foo\n"
"{\n"
"void Bar();\n"
-   "};",
+   "}; // namespace Foo",
Style);
 }
 
@@ -9872,7 +9872,7 @@
Style);
   verifyFormat("namespace Foo {\n"
"void Bar();\n"
-   "};",
+   "}; // namespace Foo",
Style);
 
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
@@ -9913,7 +9913,7 @@
   verifyFormat("namespace Foo\n"
"{\n"
"void Bar();\n"
-   "};",
+   "}; // namespace Foo",
Style);
 }
 
Index: clang/lib/Format/NamespaceEndCommentsFixer.cpp
===
---

[PATCH] D72553: [clang-tidy] Add performance-prefer-preincrement check

2020-09-13 Thread Jonas Toth via Phabricator via cfe-commits 
JonasToth accepted this revision.
JonasToth added a comment.
This revision is now accepted and ready to land.

Sorry for missing this review, i simply hadn't time :/
All my concerns are addressed and I think this is LGTM after rebasing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72553

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

[PATCH] D54222: [clang-tidy] Add a check to detect returning static locals in public headers

2020-09-13 Thread Jonas Toth via Phabricator via cfe-commits 
JonasToth added a comment.
Herald added subscribers: mgehre, Charusso.

ping.
This checks seems pretty much done. @jranieri-grammatech do you think there is 
something missing? Otherwise it could be rebased and comitted?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D54222

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

[PATCH] D87291: [clang-format][regression][PR47461] ifdef causes catch to be seen as a function

2020-09-13 Thread MyDeveloperDay via Phabricator via cfe-commits 
MyDeveloperDay updated this revision to Diff 291452.
MyDeveloperDay added a comment.

Minimize other places where try could be incorrectly seen as an identifier

  try // foo 
  {

or

  try /* foo */


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

https://reviews.llvm.org/D87291

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -2743,6 +2743,43 @@
   verifyFormat("int catch, size;");
   verifyFormat("catch = foo();");
   verifyFormat("if (catch < size) {\n  return true;\n}");
+
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterFunction = true;
+  Style.BraceWrapping.BeforeCatch = true;
+  verifyFormat("try {\n"
+   "  int bar = 1;\n"
+   "}\n"
+   "catch (...) {\n"
+   "  int bar = 1;\n"
+   "}",
+   Style);
+  verifyFormat("#if NO_EX\n"
+   "try\n"
+   "#endif\n"
+   "{\n"
+   "}\n"
+   "#if NO_EX\n"
+   "catch (...) {\n"
+   "}",
+   Style);
+  verifyFormat("try /* abc */ {\n"
+   "  int bar = 1;\n"
+   "}\n"
+   "catch (...) {\n"
+   "  int bar = 1;\n"
+   "}",
+   Style);
+  verifyFormat("try\n"
+   "// abc\n"
+   "{\n"
+   "  int bar = 1;\n"
+   "}\n"
+   "catch (...) {\n"
+   "  int bar = 1;\n"
+   "}",
+   Style);
 }
 
 TEST_F(FormatTest, FormatSEHTryCatch) {
Index: clang/lib/Format/FormatTokenLexer.cpp
===
--- clang/lib/Format/FormatTokenLexer.cpp
+++ clang/lib/Format/FormatTokenLexer.cpp
@@ -401,7 +401,7 @@
   if (!Try->is(tok::kw_try))
 return false;
   auto  = *(Tokens.end() - 1);
-  if (Next->isOneOf(tok::l_brace, tok::colon))
+  if (Next->isOneOf(tok::l_brace, tok::colon, tok::hash, tok::comment))
 return false;
 
   if (Tokens.size() > 2) {


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -2743,6 +2743,43 @@
   verifyFormat("int catch, size;");
   verifyFormat("catch = foo();");
   verifyFormat("if (catch < size) {\n  return true;\n}");
+
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterFunction = true;
+  Style.BraceWrapping.BeforeCatch = true;
+  verifyFormat("try {\n"
+   "  int bar = 1;\n"
+   "}\n"
+   "catch (...) {\n"
+   "  int bar = 1;\n"
+   "}",
+   Style);
+  verifyFormat("#if NO_EX\n"
+   "try\n"
+   "#endif\n"
+   "{\n"
+   "}\n"
+   "#if NO_EX\n"
+   "catch (...) {\n"
+   "}",
+   Style);
+  verifyFormat("try /* abc */ {\n"
+   "  int bar = 1;\n"
+   "}\n"
+   "catch (...) {\n"
+   "  int bar = 1;\n"
+   "}",
+   Style);
+  verifyFormat("try\n"
+   "// abc\n"
+   "{\n"
+   "  int bar = 1;\n"
+   "}\n"
+   "catch (...) {\n"
+   "  int bar = 1;\n"
+   "}",
+   Style);
 }
 
 TEST_F(FormatTest, FormatSEHTryCatch) {
Index: clang/lib/Format/FormatTokenLexer.cpp
===
--- clang/lib/Format/FormatTokenLexer.cpp
+++ clang/lib/Format/FormatTokenLexer.cpp
@@ -401,7 +401,7 @@
   if (!Try->is(tok::kw_try))
 return false;
   auto  = *(Tokens.end() - 1);
-  if (Next->isOneOf(tok::l_brace, tok::colon))
+  if (Next->isOneOf(tok::l_brace, tok::colon, tok::hash, tok::comment))
 return false;
 
   if (Tokens.size() > 2) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75107: [clang-tidy] hicpp-signed-bitwise IgnorePositiveIntegerLiterals now partially recursive

2020-09-13 Thread Jonas Toth via Phabricator via cfe-commits 
JonasToth added a comment.

ping.
@njames93 are you revisiting this patch? What is the missing piece? Maybe i can 
help out a bit?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75107

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

[PATCH] D76211: OpenMP Metadirective with user defined condition

2020-09-13 Thread Alok Mishra via Phabricator via cfe-commits 
alokmishra.besu updated this revision to Diff 291449.
alokmishra.besu added a comment.

Updated patch to make it extendable to more selectors
Implemented device_arch and implementation_vendor selectors as well


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76211

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/StmtOpenMP.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/StmtNodes.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/AST/StmtOpenMP.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/Basic/OpenMPKinds.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaExceptionSpec.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/test/OpenMP/metadirective_ast_print.cpp
  clang/test/OpenMP/metadirective_codegen.cpp
  clang/tools/libclang/CIndex.cpp
  clang/tools/libclang/CXCursor.cpp
  llvm/include/llvm/Frontend/OpenMP/OMP.td
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def

Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -111,6 +111,7 @@
 __OMP_CLAUSE(uses_allocators, OMPUsesAllocatorsClause)
 __OMP_CLAUSE(affinity, OMPAffinityClause)
 __OMP_CLAUSE(use_device_addr, OMPUseDeviceAddrClause)
+__OMP_CLAUSE(when,  OMPWhenClause)
 
 __OMP_CLAUSE_NO_CLASS(uniform)
 __OMP_CLAUSE_NO_CLASS(device_type)
Index: llvm/include/llvm/Frontend/OpenMP/OMP.td
===
--- llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -59,6 +59,9 @@
   let clangClass = "OMPCollapseClause";
   let flangClassValue = "ScalarIntConstantExpr";
 }
+def OMPC_When: Clause<"when"> {
+  let clangClass = "OMPWhenClause";
+}
 def OMPC_Default : Clause<"default"> {
   let clangClass = "OMPDefaultClause";
   let flangClass = "OmpDefaultClause";
@@ -294,6 +297,14 @@
 // Definition of OpenMP directives
 //===--===//
 
+def OMP_Metadirective : Directive<"metadirective"> {
+  let allowedClauses = [
+VersionedClause
+  ];
+  let allowedOnceClauses = [
+VersionedClause
+  ];
+}
 def OMP_ThreadPrivate : Directive<"threadprivate"> {}
 def OMP_Parallel : Directive<"parallel"> {
   let allowedClauses = [
Index: clang/tools/libclang/CXCursor.cpp
===
--- clang/tools/libclang/CXCursor.cpp
+++ clang/tools/libclang/CXCursor.cpp
@@ -639,6 +639,9 @@
   case Stmt::MSDependentExistsStmtClass:
 K = CXCursor_UnexposedStmt;
 break;
+  case Stmt::OMPMetaDirectiveClass:
+K = CXCursor_OMPMetaDirective;
+break;
   case Stmt::OMPParallelDirectiveClass:
 K = CXCursor_OMPParallelDirective;
 break;
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -2219,6 +2219,10 @@
   Visitor->AddStmt(C->getNumForLoops());
 }
 
+void OMPClauseEnqueue::VisitOMPWhenClause(const OMPWhenClause *C) {
+  Visitor->AddStmt(C->getDirective());
+}
+
 void OMPClauseEnqueue::VisitOMPDefaultClause(const OMPDefaultClause *C) {}
 
 void OMPClauseEnqueue::VisitOMPProcBindClause(const OMPProcBindClause *C) {}
@@ -5524,6 +5528,8 @@
 return cxstring::createRef("CXXAccessSpecifier");
   case CXCursor_ModuleImportDecl:
 return cxstring::createRef("ModuleImport");
+  case CXCursor_OMPMetaDirective:
+return cxstring::createRef("OMPMetaDirective");
   case CXCursor_OMPParallelDirective:
 return cxstring::createRef("OMPParallelDirective");
   case CXCursor_OMPSimdDirective:
Index: clang/test/OpenMP/metadirective_codegen.cpp
===
--- /dev/null
+++ clang/test/OpenMP/metadirective_codegen.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -verify -fopenmp -emit-llvm %s -o - | FileCheck %s
+// expected-no-diagnostics
+
+int main(int argc, char **argv) {
+  int N = 15;
+#pragma omp metadirective when(user = {condition(N <= 1)} \
+   : parallel)\
+when(user =

[clang] 63182c2 - [gcov] Add spanning tree optimization

2020-09-13 Thread Fangrui Song via cfe-commits 

Author: Fangrui Song
Date: 2020-09-13T00:07:31-07:00
New Revision: 63182c2ac0b643a60d397274e8a31166fc7243fa

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

LOG: [gcov] Add spanning tree optimization

gcov is an "Edge Profiling with Edge Counters" application according to
Optimally Profiling and Tracing Programs (1994).

The minimum number of counters necessary is |E|-(|V|-1). The unmeasured edges
form a spanning tree. Both GCC --coverage and clang -fprofile-generate leverage
this optimization. This patch implements the optimization for clang --coverage.
The produced .gcda files are much smaller now.

Added: 
llvm/test/Transforms/GCOVProfiling/split-indirectbr-critical-edges.ll

Modified: 
clang/test/CodeGen/code-coverage-tsan.c
compiler-rt/test/profile/Posix/gcov-fork.c
compiler-rt/test/profile/gcov-dump-and-remove.c
llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp
llvm/test/Transforms/GCOVProfiling/atomic-counter.ll

Removed: 




diff  --git a/clang/test/CodeGen/code-coverage-tsan.c 
b/clang/test/CodeGen/code-coverage-tsan.c
index 023a99598075..17f6596aa83d 100644
--- a/clang/test/CodeGen/code-coverage-tsan.c
+++ b/clang/test/CodeGen/code-coverage-tsan.c
@@ -5,7 +5,6 @@
 // CHECK-LABEL: void @foo()
 /// Two counters are incremented by __tsan_atomic64_fetch_add.
 // CHECK: call i64 @__tsan_atomic64_fetch_add
-// CHECK-NEXT:call i64 @__tsan_atomic64_fetch_add
 // CHECK-NEXT:call i32 @__tsan_atomic32_fetch_sub
 
 _Atomic(int) cnt;

diff  --git a/compiler-rt/test/profile/Posix/gcov-fork.c 
b/compiler-rt/test/profile/Posix/gcov-fork.c
index b89eb64922f0..e66690a961e2 100644
--- a/compiler-rt/test/profile/Posix/gcov-fork.c
+++ b/compiler-rt/test/profile/Posix/gcov-fork.c
@@ -17,7 +17,7 @@ int main(void) {   // CHECK-NEXT: 1: 
[[#@LINE]]:
   int status;  // CHECK-NEXT: -: [[#@LINE]]:
   func1(); // CHECK-NEXT: 1: [[#@LINE]]:
   pid_t pid = fork();  // CHECK-NEXT: 1: [[#@LINE]]:
-  if (pid == -1) return 1; // CHECK-NEXT: 2: [[#@LINE]]:
+  if (pid == -1) return 1; // CHECK-NEXT: 1: [[#@LINE]]:
   if (pid) // CHECK-NEXT: 2: [[#@LINE]]:
 wait(); // CHECK-NEXT: 1: [[#@LINE]]:
   func2(); // CHECK-NEXT: 2: [[#@LINE]]:

diff  --git a/compiler-rt/test/profile/gcov-dump-and-remove.c 
b/compiler-rt/test/profile/gcov-dump-and-remove.c
index b7f80535aada..c35640f93b3d 100644
--- a/compiler-rt/test/profile/gcov-dump-and-remove.c
+++ b/compiler-rt/test/profile/gcov-dump-and-remove.c
@@ -11,10 +11,10 @@
 extern void __gcov_dump(void);
 extern void __gcov_reset(void);
 extern int remove(const char *);   // CHECK:  -: [[#@LINE]]:extern int 
remove
-int main(void) {   // CHECK-NEXT: #: [[#@LINE]]:
-  __gcov_dump();   // CHECK-NEXT: #: [[#@LINE]]:
-  __gcov_reset();  // CHECK-NEXT: #: [[#@LINE]]:
-  if (remove("gcov-dump-and-remove.gcda") != 0) // CHECK-NEXT: #: 
[[#@LINE]]:
+int main(void) {   // CHECK-NEXT: 1: [[#@LINE]]:
+  __gcov_dump();   // CHECK-NEXT: 1: [[#@LINE]]:
+  __gcov_reset();  // CHECK-NEXT: 1: [[#@LINE]]:
+  if (remove("gcov-dump-and-remove.gcda") != 0) // CHECK-NEXT: 1: 
[[#@LINE]]:
 return 1;  // CHECK-NEXT: #: [[#@LINE]]: return 1;
// CHECK-NEXT: -: [[#@LINE]]:
   __gcov_dump();   // CHECK-NEXT: 1: [[#@LINE]]:

diff  --git a/llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp 
b/llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp
index 734deda99707..437063eef6f9 100644
--- a/llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp
+++ b/llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp
@@ -13,6 +13,7 @@
 //
 
//===--===//
 
+#include "CFGMST.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/STLExtras.h"
@@ -20,6 +21,8 @@
 #include "llvm/ADT/Statistic.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/Analysis/BlockFrequencyInfo.h"
+#include "llvm/Analysis/BranchProbabilityInfo.h"
 #include "llvm/Analysis/EHPersonalities.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/IR/CFG.h"
@@ -53,6 +56,8 @@ namespace endian = llvm::support::endian;
 #define DEBUG_TYPE "insert-gcov-profiling"
 
 enum : uint32_t {
+  GCOV_ARC_ON_TREE = 1 << 0,
+
   GCOV_TAG_FUNCTION = 0x0100,
   GCOV_TAG_BLOCKS = 0x0141,
   GCOV_TAG_ARCS = 0x0143,
@@ -94,9 +99,10 @@ class GCOVProfiler {
 public:
   GCOVProfiler() :