from:"Jeremy Demeule via Phabricator via cfe\-commits"

[PATCH] D69282: [RFC] Add a VCS conflict marker format printing on tooling::ReplacementError

2020-01-19 Thread Jeremy Demeule via Phabricator via cfe-commits

jdemeule added a comment.

Kindly ping reviewers.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69282



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

[PATCH] D69475: [clang] Provide better fix-it on exception spec error

2020-01-19 Thread Jeremy Demeule via Phabricator via cfe-commits

jdemeule added a comment.

Kindly ping reviewer.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69475



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

[PATCH] D69475: [clang] Provide better fix-it on exception spec error

2019-12-14 Thread Jeremy Demeule via Phabricator via cfe-commits

jdemeule added a comment.

Kindly ping reviewer.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69475



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

[PATCH] D69282: [RFC] Add a VCS conflict marker format printing on tooling::ReplacementError

2019-12-14 Thread Jeremy Demeule via Phabricator via cfe-commits

jdemeule added a comment.

Kindly ping reviewers.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69282



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

[PATCH] D69475: [clang] Provide better fix-it on exception spec error

2019-11-22 Thread Jeremy Demeule via Phabricator via cfe-commits

jdemeule added a comment.

Kindly ping reviewer.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69475



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

[PATCH] D69282: [RFC] Add a VCS conflict marker format printing on tooling::ReplacementError

2019-11-22 Thread Jeremy Demeule via Phabricator via cfe-commits

jdemeule added a reviewer: ioeric.
jdemeule added a comment.

Kindly ping reviewers and adding Eric as you were interested last year about 
that.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69282



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

[PATCH] D69475: [clang] Provide better fix-it on exception spec error

2019-10-27 Thread Jeremy Demeule via Phabricator via cfe-commits

jdemeule created this revision.
jdemeule added a reviewer: rsmith.
jdemeule added a project: clang.

Clang will report an error when the exception specification is not respected 
with a fix-it.
However, this fix-it does not fully qualify the type in case of function 
template specialization which lead to "incomplete" transformation (resulting 
code does not compile).
This patch attempts to create a more realiable in the first place by reusing 
the pretty printer helper defined in `SemaTemplate` which expand template 
arguments.


Repository:
  rC Clang

https://reviews.llvm.org/D69475

Files:
  clang/include/clang/AST/PrettyPrinter.h
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/Sema/SemaExceptionSpec.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/SemaCXX/exception-spec.cpp

Index: clang/test/SemaCXX/exception-spec.cpp
===
--- clang/test/SemaCXX/exception-spec.cpp
+++ clang/test/SemaCXX/exception-spec.cpp
@@ -52,3 +52,21 @@
 D2 =(const D2&); // expected-error {{more lax}}
   };
 }
+
+namespace MissingOnTemplate {
+  template
+  struct type_dependant_noexcept {
+static constexpr const bool value = (sizeof(T) < 4);
+  };
+
+  template void a_fct(T) noexcept(type_dependant_noexcept::value);  // expected-note {{previous}}
+  template void a_fct(T);  // expected-error {{missing exception specification 'noexcept(type_dependant_noexcept::value)'}}
+}
+
+namespace abc {
+  struct A {};
+}  // namespace abc
+
+namespace MissingOnTemplate {
+  template<>void a_fct(abc::A);  // expected-error {{missing exception specification 'noexcept(type_dependant_noexcept::value)'}} expected-note {{previous}}
+}
\ No newline at end of file
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -3115,40 +3115,6 @@
   return Cond;
 }
 
-namespace {
-
-// A PrinterHelper that prints more helpful diagnostics for some sub-expressions
-// within failing boolean expression, such as substituting template parameters
-// for actual types.
-class FailedBooleanConditionPrinterHelper : public PrinterHelper {
-public:
-  explicit FailedBooleanConditionPrinterHelper(const PrintingPolicy )
-  : Policy(P) {}
-
-  bool handledStmt(Stmt *E, raw_ostream ) override {
-const auto *DR = dyn_cast(E);
-if (DR && DR->getQualifier()) {
-  // If this is a qualified name, expand the template arguments in nested
-  // qualifiers.
-  DR->getQualifier()->print(OS, Policy, true);
-  // Then print the decl itself.
-  const ValueDecl *VD = DR->getDecl();
-  OS << VD->getName();
-  if (const auto *IV = dyn_cast(VD)) {
-// This is a template variable, print the expanded template arguments.
-printTemplateArgumentList(OS, IV->getTemplateArgs().asArray(), Policy);
-  }
-  return true;
-}
-return false;
-  }
-
-private:
-  const PrintingPolicy Policy;
-};
-
-} // end anonymous namespace
-
 std::pair
 Sema::findFailedBooleanCondition(Expr *Cond) {
   Cond = lookThroughRangesV3Condition(PP, Cond);
@@ -3187,7 +3153,7 @@
 llvm::raw_string_ostream Out(Description);
 PrintingPolicy Policy = getPrintingPolicy();
 Policy.PrintCanonicalTypes = true;
-FailedBooleanConditionPrinterHelper Helper(Policy);
+ExpandTemplateArgumentsPrinterHelper Helper(Policy);
 FailedCond->printPretty(Out, , Policy, 0, "\n", nullptr);
   }
   return { FailedCond, Description };
Index: clang/lib/Sema/SemaExceptionSpec.cpp
===
--- clang/lib/Sema/SemaExceptionSpec.cpp
+++ clang/lib/Sema/SemaExceptionSpec.cpp
@@ -423,7 +423,11 @@
   case EST_NoexceptTrue:
 OS << "noexcept(";
 assert(OldProto->getNoexceptExpr() != nullptr && "Expected non-null Expr");
-OldProto->getNoexceptExpr()->printPretty(OS, nullptr, getPrintingPolicy());
+{
+  PrintingPolicy Policy = getPrintingPolicy();
+  ExpandTemplateArgumentsPrinterHelper Helper(Policy);
+  OldProto->getNoexceptExpr()->printPretty(OS, , Policy);
+}
 OS << ")";
 break;
   case EST_NoThrow:
Index: clang/lib/AST/StmtPrinter.cpp
===
--- clang/lib/AST/StmtPrinter.cpp
+++ clang/lib/AST/StmtPrinter.cpp
@@ -2472,3 +2472,26 @@
 
 // Implement virtual destructor.
 PrinterHelper::~PrinterHelper() = default;
+
+//===--===//
+// ExpandTemplateArgumentsPrinterHelper
+//===--===//
+
+bool ExpandTemplateArgumentsPrinterHelper::handledStmt(Stmt *E,
+   raw_ostream ) {
+  const auto *DR = dyn_cast(E);
+  if (DR && DR->getQualifier()) {
+// If this is a qualified name, expand the template arguments in nested
+// qualifiers.
+

[PATCH] D69282: [RFC] Add a VCS conflict marker format printing on tooling::ReplacementError

2019-10-21 Thread Jeremy Demeule via Phabricator via cfe-commits

jdemeule created this revision.
jdemeule added a reviewer: klimek.
Herald added subscribers: cfe-commits, jfb.
Herald added a project: clang.

Last year, I made some suggestion to improve conflict reporting in 
clang-apply-replacements (https://reviews.llvm.org/D43764).
I have a very simple implementation of a VCS conflict marker to share and get 
some feedbacks if this approach could get some interests.


Repository:
  rC Clang

https://reviews.llvm.org/D69282

Files:
  clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
  clang-tools-extra/test/clang-apply-replacements/Inputs/conflict/expected.txt
  
clang-tools-extra/test/clang-apply-replacements/Inputs/order-dependent/expected.txt
  clang/include/clang/Tooling/Core/Replacement.h
  clang/lib/Tooling/Core/Replacement.cpp
  clang/unittests/Tooling/RefactoringTest.cpp

Index: clang/unittests/Tooling/RefactoringTest.cpp
===
--- clang/unittests/Tooling/RefactoringTest.cpp
+++ clang/unittests/Tooling/RefactoringTest.cpp
@@ -24,10 +24,13 @@
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Rewrite/Core/Rewriter.h"
+#include "clang/Tooling/Core/Replacement.h"
 #include "clang/Tooling/Refactoring/AtomicChange.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/SmallString.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/VirtualFileSystem.h"
+#include "llvm/Support/raw_ostream.h"
 #include "gtest/gtest.h"
 
 namespace clang {
@@ -58,8 +61,8 @@
 }
 
 TEST_F(ReplacementTest, CanReplaceTextAtPosition) {
-  FileID ID = Context.createInMemoryFile("input.cpp",
- "line1\nline2\nline3\nline4");
+  FileID ID =
+  Context.createInMemoryFile("input.cpp", "line1\nline2\nline3\nline4");
   SourceLocation Location = Context.getLocation(ID, 2, 3);
   Replacement Replace(createReplacement(Location, 12, "x"));
   EXPECT_TRUE(Replace.apply(Context.Rewrite));
@@ -67,8 +70,8 @@
 }
 
 TEST_F(ReplacementTest, CanReplaceTextAtPositionMultipleTimes) {
-  FileID ID = Context.createInMemoryFile("input.cpp",
- "line1\nline2\nline3\nline4");
+  FileID ID =
+  Context.createInMemoryFile("input.cpp", "line1\nline2\nline3\nline4");
   SourceLocation Location1 = Context.getLocation(ID, 2, 3);
   Replacement Replace1(createReplacement(Location1, 12, "x\ny\n"));
   EXPECT_TRUE(Replace1.apply(Context.Rewrite));
@@ -135,7 +138,8 @@
 }
   });
   OS.flush();
-  if (ErrorMessage.empty()) return true;
+  if (ErrorMessage.empty())
+return true;
   llvm::errs() << ErrorMessage;
   return false;
 }
@@ -430,8 +434,8 @@
 }
 
 TEST_F(ReplacementTest, CanApplyReplacements) {
-  FileID ID = Context.createInMemoryFile("input.cpp",
- "line1\nline2\nline3\nline4");
+  FileID ID =
+  Context.createInMemoryFile("input.cpp", "line1\nline2\nline3\nline4");
   Replacements Replaces =
   toReplacements({Replacement(Context.Sources,
   Context.getLocation(ID, 2, 1), 5, "replaced"),
@@ -444,8 +448,8 @@
 // Verifies that replacement/deletion is applied before insertion at the same
 // offset.
 TEST_F(ReplacementTest, InsertAndDelete) {
-  FileID ID = Context.createInMemoryFile("input.cpp",
- "line1\nline2\nline3\nline4");
+  FileID ID =
+  Context.createInMemoryFile("input.cpp", "line1\nline2\nline3\nline4");
   Replacements Replaces = toReplacements(
   {Replacement(Context.Sources, Context.getLocation(ID, 2, 1), 6, ""),
Replacement(Context.Sources, Context.getLocation(ID, 2, 1), 0,
@@ -455,8 +459,7 @@
 }
 
 TEST_F(ReplacementTest, AdjacentReplacements) {
-  FileID ID = Context.createInMemoryFile("input.cpp",
- "ab");
+  FileID ID = Context.createInMemoryFile("input.cpp", "ab");
   Replacements Replaces = toReplacements(
   {Replacement(Context.Sources, Context.getLocation(ID, 1, 1), 1, "x"),
Replacement(Context.Sources, Context.getLocation(ID, 1, 2), 1, "y")});
@@ -465,8 +468,8 @@
 }
 
 TEST_F(ReplacementTest, AddDuplicateReplacements) {
-  FileID ID = Context.createInMemoryFile("input.cpp",
- "line1\nline2\nline3\nline4");
+  FileID ID =
+  Context.createInMemoryFile("input.cpp", "line1\nline2\nline3\nline4");
   auto Replaces = toReplacements({Replacement(
   Context.Sources, Context.getLocation(ID, 2, 1), 5, "replaced")});
 
@@ -485,8 +488,8 @@
 }
 
 TEST_F(ReplacementTest, FailOrderDependentReplacements) {
-  FileID ID = Context.createInMemoryFile("input.cpp",
- "line1\nline2\nline3\nline4");
+  FileID ID =
+  Context.createInMemoryFile("input.cpp", "line1\nline2\nline3\nline4");
   auto Replaces = toReplacements({Replacement(
   Context.Sources, Context.getLocation(ID, 2, 1),

[PATCH] D43500: [clang-tidy]: modernize-use-default-member-init: Remove trailing comma and colon.

2019-06-17 Thread Jeremy Demeule via Phabricator via cfe-commits

jdemeule added a comment.

In D43500#1546089 , @lebedev.ri wrote:

> In D43500#1546077 , @jdemeule wrote:
>
> > In D43500#1244518 , @JonasToth 
> > wrote:
> >
> > > What is the status here? Will you continue to work on this patch 
> > > @jdemeule (which would be great!) ?
> >
> >
> > Hi, now, this patch contains only tests which are already covered with lit 
> > tests.
> >  The cleanup is now applied in clang-apply-replacements.
>
>
> How does that work with `clang-tidy -fix` ?


If I remember by calling 'format::cleanupAroundReplacements' when applying the 
fix in memory.


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

https://reviews.llvm.org/D43500



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

[PATCH] D43500: [clang-tidy]: modernize-use-default-member-init: Remove trailing comma and colon.

2019-06-17 Thread Jeremy Demeule via Phabricator via cfe-commits

jdemeule added a comment.

In D43500#1244518 , @JonasToth wrote:

> What is the status here? Will you continue to work on this patch @jdemeule 
> (which would be great!) ?


Hi, now, this patch contains only tests which are already covered with lit 
tests.
The cleanup is now applied in clang-apply-replacements.

In D43500#1491369 , @chgans wrote:

> Hi, Using llvm-8 (llvm ubuntu repo), clang-replacement has improved, but 
> there still a case when a comma is left beyond: if there was only one member 
> init.


I do not reproduce at master (9.0.0 trunk), do you have an example in mind?


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

https://reviews.llvm.org/D43500



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

[PATCH] D48523: [clang-tidy] Update run-clang-tidy.py with vfsoverlay arg

2018-07-10 Thread Jeremy Demeule via Phabricator via cfe-commits

jdemeule added a comment.

kindly ping reviewers.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48523



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

[PATCH] D48523: [clang-tidy] Update run-clang-tidy.py with vfsoverlay arg

2018-06-23 Thread Jeremy Demeule via Phabricator via cfe-commits

jdemeule created this revision.
jdemeule added a reviewer: alexfh.
Herald added subscribers: cfe-commits, xazax.hun.

Updating the run-clang-tidy.py script to allow specification of the
vfsoverlay argument to the clang-tidy invocation.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48523

Files:
  clang-tidy/tool/run-clang-tidy.py


Index: clang-tidy/tool/run-clang-tidy.py
===
--- clang-tidy/tool/run-clang-tidy.py
+++ clang-tidy/tool/run-clang-tidy.py
@@ -75,8 +75,8 @@
 
 
 def get_tidy_invocation(f, clang_tidy_binary, checks, tmpdir, build_path,
-header_filter, extra_arg, extra_arg_before, quiet,
-config):
+vfsoverlay, header_filter, extra_arg, extra_arg_before,
+quiet, config):
   """Gets a command line for clang-tidy."""
   start = [clang_tidy_binary]
   if header_filter is not None:
@@ -98,6 +98,8 @@
   for arg in extra_arg_before:
   start.append('-extra-arg-before=%s' % arg)
   start.append('-p=' + build_path)
+  if vfsoverlay is not None:
+start.append('-vfsoverlay=' + vfsoverlay)
   if quiet:
   start.append('-quiet')
   if config:
@@ -158,9 +160,10 @@
   while True:
 name = queue.get()
 invocation = get_tidy_invocation(name, args.clang_tidy_binary, args.checks,
- tmpdir, build_path, args.header_filter,
- args.extra_arg, args.extra_arg_before,
- args.quiet, args.config)
+ tmpdir, build_path, args.vfsoverlay,
+ args.header_filter, args.extra_arg,
+ args.extra_arg_before, args.quiet,
+ args.config)
 sys.stdout.write(' '.join(invocation) + '\n')
 return_code = subprocess.call(invocation)
 if return_code != 0:
@@ -209,6 +212,9 @@
   'code after applying fixes')
   parser.add_argument('-p', dest='build_path',
   help='Path used to read a compile command database.')
+  parser.add_argument('-vfsoverlay', dest='vfsoverlay',
+  help='Overlay the virtual filesystem described by file '
+  'over the real file system.')
   parser.add_argument('-extra-arg', dest='extra_arg',
   action='append', default=[],
   help='Additional argument to append to the compiler '


Index: clang-tidy/tool/run-clang-tidy.py
===
--- clang-tidy/tool/run-clang-tidy.py
+++ clang-tidy/tool/run-clang-tidy.py
@@ -75,8 +75,8 @@
 
 
 def get_tidy_invocation(f, clang_tidy_binary, checks, tmpdir, build_path,
-header_filter, extra_arg, extra_arg_before, quiet,
-config):
+vfsoverlay, header_filter, extra_arg, extra_arg_before,
+quiet, config):
   """Gets a command line for clang-tidy."""
   start = [clang_tidy_binary]
   if header_filter is not None:
@@ -98,6 +98,8 @@
   for arg in extra_arg_before:
   start.append('-extra-arg-before=%s' % arg)
   start.append('-p=' + build_path)
+  if vfsoverlay is not None:
+start.append('-vfsoverlay=' + vfsoverlay)
   if quiet:
   start.append('-quiet')
   if config:
@@ -158,9 +160,10 @@
   while True:
 name = queue.get()
 invocation = get_tidy_invocation(name, args.clang_tidy_binary, args.checks,
- tmpdir, build_path, args.header_filter,
- args.extra_arg, args.extra_arg_before,
- args.quiet, args.config)
+ tmpdir, build_path, args.vfsoverlay,
+ args.header_filter, args.extra_arg,
+ args.extra_arg_before, args.quiet,
+ args.config)
 sys.stdout.write(' '.join(invocation) + '\n')
 return_code = subprocess.call(invocation)
 if return_code != 0:
@@ -209,6 +212,9 @@
   'code after applying fixes')
   parser.add_argument('-p', dest='build_path',
   help='Path used to read a compile command database.')
+  parser.add_argument('-vfsoverlay', dest='vfsoverlay',
+  help='Overlay the virtual filesystem described by file '
+  'over the real file system.')
   parser.add_argument('-extra-arg', dest='extra_arg',
   action='append', default=[],
   help='Additional argument to append to the compiler '
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

2018-04-11 Thread Jeremy Demeule via Phabricator via cfe-commits

jdemeule updated this revision to Diff 141969.
jdemeule marked 3 inline comments as done.
jdemeule added a comment.

Sort replacements before converting them to AtomicChange.


https://reviews.llvm.org/D43764

Files:
  
clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h
  clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
  clang-apply-replacements/tool/CMakeLists.txt
  clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
  test/clang-apply-replacements/Inputs/basic/file2.yaml
  test/clang-apply-replacements/Inputs/conflict/expected.txt
  test/clang-apply-replacements/Inputs/identical/file1.yaml
  test/clang-apply-replacements/Inputs/identical/identical.cpp
  test/clang-apply-replacements/Inputs/order-dependent/expected.txt
  test/clang-apply-replacements/Inputs/order-dependent/file1.yaml
  test/clang-apply-replacements/Inputs/order-dependent/file2.yaml
  test/clang-apply-replacements/Inputs/order-dependent/order-dependent.cpp
  test/clang-apply-replacements/identical.cpp
  test/clang-apply-replacements/order-dependent.cpp
  unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
  unittests/clang-apply-replacements/CMakeLists.txt
  unittests/clang-apply-replacements/ReformattingTest.cpp

Index: unittests/clang-apply-replacements/ReformattingTest.cpp
===
--- unittests/clang-apply-replacements/ReformattingTest.cpp
+++ /dev/null
@@ -1,67 +0,0 @@
-//===- clang-apply-replacements/ReformattingTest.cpp --===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#include "clang-apply-replacements/Tooling/ApplyReplacements.h"
-#include "common/VirtualFileHelper.h"
-#include "clang/Format/Format.h"
-#include "clang/Tooling/Refactoring.h"
-#include "gtest/gtest.h"
-
-using namespace clang;
-using namespace clang::tooling;
-using namespace clang::replace;
-
-typedef std::vector ReplacementsVec;
-
-static Replacement makeReplacement(unsigned Offset, unsigned Length,
-   unsigned ReplacementLength,
-   llvm::StringRef FilePath = "") {
-  return Replacement(FilePath, Offset, Length,
- std::string(ReplacementLength, '~'));
-}
-
-// generate a set of replacements containing one element
-static ReplacementsVec makeReplacements(unsigned Offset, unsigned Length,
-unsigned ReplacementLength,
-llvm::StringRef FilePath = "~") {
-  ReplacementsVec Replaces;
-  Replaces.push_back(
-  makeReplacement(Offset, Length, ReplacementLength, FilePath));
-  return Replaces;
-}
-
-// Put these functions in the clang::tooling namespace so arg-dependent name
-// lookup finds these functions for the EXPECT_EQ macros below.
-namespace clang {
-namespace tooling {
-
-std::ostream <<(std::ostream , const Range ) {
-  return os << "Range(" << R.getOffset() << ", " << R.getLength() << ")";
-}
-
-} // namespace tooling
-} // namespace clang
-
-// Ensure zero-length ranges are produced. Even lines where things are deleted
-// need reformatting.
-TEST(CalculateChangedRangesTest, producesZeroLengthRange) {
-  RangeVector Changes = calculateChangedRanges(makeReplacements(0, 4, 0));
-  EXPECT_EQ(Range(0, 0), Changes.front());
-}
-
-// Basic test to ensure replacements turn into ranges properly.
-TEST(CalculateChangedRangesTest, calculatesRanges) {
-  ReplacementsVec R;
-  R.push_back(makeReplacement(2, 0, 3));
-  R.push_back(makeReplacement(5, 2, 4));
-  RangeVector Changes = calculateChangedRanges(R);
-
-  Range ExpectedRanges[] = { Range(2, 3), Range(8, 4) };
-  EXPECT_TRUE(std::equal(Changes.begin(), Changes.end(), ExpectedRanges));
-}
Index: unittests/clang-apply-replacements/CMakeLists.txt
===
--- unittests/clang-apply-replacements/CMakeLists.txt
+++ unittests/clang-apply-replacements/CMakeLists.txt
@@ -9,12 +9,12 @@
 
 add_extra_unittest(ClangApplyReplacementsTests
   ApplyReplacementsTest.cpp
-  ReformattingTest.cpp
   )
 
 target_link_libraries(ClangApplyReplacementsTests
   PRIVATE
   clangApplyReplacements
   clangBasic
   clangToolingCore
+  clangToolingRefactor
   )
Index: unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
===
--- unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
+++ unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
@@ -9,6 +9,7 @@
 //===--===//
 
 #include "clang-apply-replacements/Tooling/ApplyReplacements.h"
+#include "clang/Format/Format.h"
 #include "gtest/gtest.h"
 
 using

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

2018-04-10 Thread Jeremy Demeule via Phabricator via cfe-commits

jdemeule added a comment.

Ok, I understand the problem.
Previously, during the deduplication step, replacements per file where sorted 
and it is not the case anymore.
That means now, clang-apply-replacement is highly dependant of reading file 
order during error reporting.
I mainly see 2 options (I can miss others), rewrite the test (e.g merging yaml 
file together) or sort the replacements per file to fix the order of 
application.

Can I ask you what solution you prefer?


https://reviews.llvm.org/D43764



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

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

2018-04-08 Thread Jeremy Demeule via Phabricator via cfe-commits

jdemeule added a comment.

Thank you for applying the patch.

I suspect some undefined order when applying replacements as directly dependant 
in which order OS reads `file1.yaml`, `file2.yaml`and `file3.yaml`.
As I am not able to reproduce it on my machine, can you share with me the 
content of `output.txt`?


https://reviews.llvm.org/D43764



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

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

2018-04-05 Thread Jeremy Demeule via Phabricator via cfe-commits

jdemeule added a comment.

Can I help in some way to close this PR?


https://reviews.llvm.org/D43764



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

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

2018-03-28 Thread Jeremy Demeule via Phabricator via cfe-commits

jdemeule added a comment.

Thanks a lot!
Can I ask you if I need to do something as I do not have commit access?


https://reviews.llvm.org/D43764



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

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

2018-03-27 Thread Jeremy Demeule via Phabricator via cfe-commits

jdemeule updated this revision to Diff 139889.
jdemeule added a comment.

Update after review (cleanup and improve comments).


https://reviews.llvm.org/D43764

Files:
  
clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h
  clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
  clang-apply-replacements/tool/CMakeLists.txt
  clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
  test/clang-apply-replacements/Inputs/basic/file2.yaml
  test/clang-apply-replacements/Inputs/conflict/expected.txt
  test/clang-apply-replacements/Inputs/identical/file1.yaml
  test/clang-apply-replacements/Inputs/identical/identical.cpp
  test/clang-apply-replacements/Inputs/order-dependent/expected.txt
  test/clang-apply-replacements/Inputs/order-dependent/file1.yaml
  test/clang-apply-replacements/Inputs/order-dependent/file2.yaml
  test/clang-apply-replacements/Inputs/order-dependent/order-dependent.cpp
  test/clang-apply-replacements/identical.cpp
  test/clang-apply-replacements/order-dependent.cpp
  unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
  unittests/clang-apply-replacements/CMakeLists.txt
  unittests/clang-apply-replacements/ReformattingTest.cpp

Index: unittests/clang-apply-replacements/ReformattingTest.cpp
===
--- unittests/clang-apply-replacements/ReformattingTest.cpp
+++ /dev/null
@@ -1,67 +0,0 @@
-//===- clang-apply-replacements/ReformattingTest.cpp --===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#include "clang-apply-replacements/Tooling/ApplyReplacements.h"
-#include "common/VirtualFileHelper.h"
-#include "clang/Format/Format.h"
-#include "clang/Tooling/Refactoring.h"
-#include "gtest/gtest.h"
-
-using namespace clang;
-using namespace clang::tooling;
-using namespace clang::replace;
-
-typedef std::vector ReplacementsVec;
-
-static Replacement makeReplacement(unsigned Offset, unsigned Length,
-   unsigned ReplacementLength,
-   llvm::StringRef FilePath = "") {
-  return Replacement(FilePath, Offset, Length,
- std::string(ReplacementLength, '~'));
-}
-
-// generate a set of replacements containing one element
-static ReplacementsVec makeReplacements(unsigned Offset, unsigned Length,
-unsigned ReplacementLength,
-llvm::StringRef FilePath = "~") {
-  ReplacementsVec Replaces;
-  Replaces.push_back(
-  makeReplacement(Offset, Length, ReplacementLength, FilePath));
-  return Replaces;
-}
-
-// Put these functions in the clang::tooling namespace so arg-dependent name
-// lookup finds these functions for the EXPECT_EQ macros below.
-namespace clang {
-namespace tooling {
-
-std::ostream <<(std::ostream , const Range ) {
-  return os << "Range(" << R.getOffset() << ", " << R.getLength() << ")";
-}
-
-} // namespace tooling
-} // namespace clang
-
-// Ensure zero-length ranges are produced. Even lines where things are deleted
-// need reformatting.
-TEST(CalculateChangedRangesTest, producesZeroLengthRange) {
-  RangeVector Changes = calculateChangedRanges(makeReplacements(0, 4, 0));
-  EXPECT_EQ(Range(0, 0), Changes.front());
-}
-
-// Basic test to ensure replacements turn into ranges properly.
-TEST(CalculateChangedRangesTest, calculatesRanges) {
-  ReplacementsVec R;
-  R.push_back(makeReplacement(2, 0, 3));
-  R.push_back(makeReplacement(5, 2, 4));
-  RangeVector Changes = calculateChangedRanges(R);
-
-  Range ExpectedRanges[] = { Range(2, 3), Range(8, 4) };
-  EXPECT_TRUE(std::equal(Changes.begin(), Changes.end(), ExpectedRanges));
-}
Index: unittests/clang-apply-replacements/CMakeLists.txt
===
--- unittests/clang-apply-replacements/CMakeLists.txt
+++ unittests/clang-apply-replacements/CMakeLists.txt
@@ -9,12 +9,12 @@
 
 add_extra_unittest(ClangApplyReplacementsTests
   ApplyReplacementsTest.cpp
-  ReformattingTest.cpp
   )
 
 target_link_libraries(ClangApplyReplacementsTests
   PRIVATE
   clangApplyReplacements
   clangBasic
   clangToolingCore
+  clangToolingRefactor
   )
Index: unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
===
--- unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
+++ unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
@@ -9,6 +9,7 @@
 //===--===//
 
 #include "clang-apply-replacements/Tooling/ApplyReplacements.h"
+#include "clang/Format/Format.h"
 #include "gtest/gtest.h"
 
 using namespace clang::replace;
@@ -41,11 +42,12 @@

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

2018-03-20 Thread Jeremy Demeule via Phabricator via cfe-commits

jdemeule added inline comments.



Comment at: 
clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h:75
+/// \brief Deduplicate, check for conflicts, and convert all Replacements 
stored
+/// in \c TUs to AtomicChange. If conflicts occur, no Replacements are applied.
 ///

ioeric wrote:
> ` If conflicts occur, no Replacements are applied.` This doesn't seem to be 
> accurate; non-conflicting replacements are still added.
You are perfectly right the description does not match the behavior.

I think I will update this part to //"Deduplicate, check for conflicts, and 
convert all Replacements stored in TUs to a single AtomicChange per file. 
Conflicting replacement are skipped. However, the order of converting 
Replacements extracted from TUs and TUDs to AtomicChange is undefined."//

And amend `FileChanges` with //"Only non conflicting replacements are kept into 
FileChanges."//



Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:179
+  // To keep current clang-apply-replacement behavior, report conflicting
+  // replacements skip file containing conflicts, all replacements are stored
+  // into 1 big AtomicChange.

ioeric wrote:
> I think we are only skipping conflicts; non-conflicting replacements are 
> still added even if the file contains other conflicts?
> 
> This doesn't seem to have caused any problem because the current caller 
> simply drops all changes if conflict is detected in any file, but we should 
> still make it clear what the behavior of `mergeAndDeduplicate` is (in the API 
> doc) e.g.  what would `FileChanges` contain if there is conflict in some but 
> not all files?
I will update this comment to:
//"To keep current clang-apply-replacement behavior, report conflicting 
replacements on corresponding file, all replacements are stored into 1 big 
AtomicChange."//


https://reviews.llvm.org/D43764



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

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

2018-03-20 Thread Jeremy Demeule via Phabricator via cfe-commits

jdemeule updated this revision to Diff 139082.
jdemeule added a comment.

Update after review.
Change error reporting to use `ReplacementError` message.


https://reviews.llvm.org/D43764

Files:
  
clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h
  clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
  clang-apply-replacements/tool/CMakeLists.txt
  clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
  test/clang-apply-replacements/Inputs/basic/file2.yaml
  test/clang-apply-replacements/Inputs/conflict/expected.txt
  test/clang-apply-replacements/Inputs/identical/file1.yaml
  test/clang-apply-replacements/Inputs/identical/identical.cpp
  test/clang-apply-replacements/Inputs/order-dependent/expected.txt
  test/clang-apply-replacements/Inputs/order-dependent/file1.yaml
  test/clang-apply-replacements/Inputs/order-dependent/file2.yaml
  test/clang-apply-replacements/Inputs/order-dependent/order-dependent.cpp
  test/clang-apply-replacements/identical.cpp
  test/clang-apply-replacements/order-dependent.cpp
  unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
  unittests/clang-apply-replacements/CMakeLists.txt
  unittests/clang-apply-replacements/ReformattingTest.cpp

Index: unittests/clang-apply-replacements/ReformattingTest.cpp
===
--- unittests/clang-apply-replacements/ReformattingTest.cpp
+++ /dev/null
@@ -1,67 +0,0 @@
-//===- clang-apply-replacements/ReformattingTest.cpp --===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#include "clang-apply-replacements/Tooling/ApplyReplacements.h"
-#include "common/VirtualFileHelper.h"
-#include "clang/Format/Format.h"
-#include "clang/Tooling/Refactoring.h"
-#include "gtest/gtest.h"
-
-using namespace clang;
-using namespace clang::tooling;
-using namespace clang::replace;
-
-typedef std::vector ReplacementsVec;
-
-static Replacement makeReplacement(unsigned Offset, unsigned Length,
-   unsigned ReplacementLength,
-   llvm::StringRef FilePath = "") {
-  return Replacement(FilePath, Offset, Length,
- std::string(ReplacementLength, '~'));
-}
-
-// generate a set of replacements containing one element
-static ReplacementsVec makeReplacements(unsigned Offset, unsigned Length,
-unsigned ReplacementLength,
-llvm::StringRef FilePath = "~") {
-  ReplacementsVec Replaces;
-  Replaces.push_back(
-  makeReplacement(Offset, Length, ReplacementLength, FilePath));
-  return Replaces;
-}
-
-// Put these functions in the clang::tooling namespace so arg-dependent name
-// lookup finds these functions for the EXPECT_EQ macros below.
-namespace clang {
-namespace tooling {
-
-std::ostream <<(std::ostream , const Range ) {
-  return os << "Range(" << R.getOffset() << ", " << R.getLength() << ")";
-}
-
-} // namespace tooling
-} // namespace clang
-
-// Ensure zero-length ranges are produced. Even lines where things are deleted
-// need reformatting.
-TEST(CalculateChangedRangesTest, producesZeroLengthRange) {
-  RangeVector Changes = calculateChangedRanges(makeReplacements(0, 4, 0));
-  EXPECT_EQ(Range(0, 0), Changes.front());
-}
-
-// Basic test to ensure replacements turn into ranges properly.
-TEST(CalculateChangedRangesTest, calculatesRanges) {
-  ReplacementsVec R;
-  R.push_back(makeReplacement(2, 0, 3));
-  R.push_back(makeReplacement(5, 2, 4));
-  RangeVector Changes = calculateChangedRanges(R);
-
-  Range ExpectedRanges[] = { Range(2, 3), Range(8, 4) };
-  EXPECT_TRUE(std::equal(Changes.begin(), Changes.end(), ExpectedRanges));
-}
Index: unittests/clang-apply-replacements/CMakeLists.txt
===
--- unittests/clang-apply-replacements/CMakeLists.txt
+++ unittests/clang-apply-replacements/CMakeLists.txt
@@ -9,12 +9,12 @@
 
 add_extra_unittest(ClangApplyReplacementsTests
   ApplyReplacementsTest.cpp
-  ReformattingTest.cpp
   )
 
 target_link_libraries(ClangApplyReplacementsTests
   PRIVATE
   clangApplyReplacements
   clangBasic
   clangToolingCore
+  clangToolingRefactor
   )
Index: unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
===
--- unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
+++ unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
@@ -9,6 +9,7 @@
 //===--===//
 
 #include "clang-apply-replacements/Tooling/ApplyReplacements.h"
+#include "clang/Format/Format.h"
 #include "gtest/gtest.h"
 
 using namespace clang::replace;

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

2018-03-16 Thread Jeremy Demeule via Phabricator via cfe-commits

jdemeule added inline comments.



Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:353
+const FileEntry *Entry = FileAndReplacements.first;
+ReplacementsToAtomicChanges DeduplicatedChanges(SM, Entry);
+for (const auto  : FileAndReplacements.second)

ioeric wrote:
> jdemeule wrote:
> > ioeric wrote:
> > > jdemeule wrote:
> > > > ioeric wrote:
> > > > > Sorry that I didn't make myself clear... what you had in the previous 
> > > > > revision was correct (for the current use case of 
> > > > > apply-all-replacements) i.e. store all replacements in one 
> > > > > `AtomicChange`, which allows you to detect conflicts on the fly. So 
> > > > > the code here can be simplified as:
> > > > > 
> > > > > ```
> > > > > ...
> > > > > Entry = ...;
> > > > > AtomicChange FileChange;
> > > > > for (const auto  : FileAndReplacements.second) {
> > > > >   auto Err = FileChange.replace(  );
> > > > >   if (Err)
> > > > > reportConflict(Entry, std::move(Err));  // reportConflict as we go
> > > > > }
> > > > > FileChanges.emplace(Entry, {FileChange});
> > > > > ...
> > > > > ```
> > > > > 
> > > > > I think with this set up, you wouldn't need 
> > > > > `ReplacementsToAtomicChanges`, `ConflictError` and `reportConflicts`.
> > > > I think I have over-interpreting your previous comment :-)
> > > > However, I am still confused about error reporting.
> > > > Currently, clang-apply-replacements reports conflicting replacement per 
> > > > file and per conflict.
> > > > For example:
> > > > ```
> > > > There are conflicting changes to XXX:
> > > > The following changes conflict:
> > > >   Replace 8:8-8:33 with "AAA"
> > > >   Replace 8:8-8:33 with "BBB"
> > > >   Remove 8:10-8:15
> > > >   Insert at 8:14 CCC
> > > > ```
> > > > 
> > > > With this patch, conflict are reported by pair (first 
> > > > replacement/conflicted one) and I am able to print the corresponding 
> > > > file only once (thanks to the defered reporting).
> > > > ```
> > > > There are conflicting changes to XXX:
> > > > The following changes conflict:
> > > >   Replace 8:8-8:33 with "AAA"
> > > >   Replace 8:8-8:33 with "BBB"
> > > > The following changes conflict:
> > > >   Replace 8:8-8:33 with "AAA"
> > > >   Remove 8:10-8:15
> > > > The following changes conflict:
> > > >   Replace 8:8-8:33 with "AAA"
> > > >   Insert at 8:14 CCC
> > > > ```
> > > > 
> > > > I prefer the way you suggest to report conflict but we will loose or 
> > > > print conflicting file at each conflict detected.
> > > > I even more prefer to use `llvm::toString(std::move(Err))` but this 
> > > > will fully change the reporting and will also be reported by pair.
> > > > ```
> > > > The new replacement overlaps with an existing replacement.
> > > > New replacement: XXX: 106:+26:"AAA"
> > > > Existing replacement: XXX: 106:+26:"BBB"
> > > > The new replacement overlaps with an existing replacement.
> > > > New replacement: XXX: 106:+26:"AAA"
> > > > Existing replacement: XXX: 112:+0:"CCC"
> > > > The new replacement overlaps with an existing replacement.
> > > > New replacement: XXX: 106:+26:"AAA"
> > > > Existing replacement: XXX: 108:+12:""
> > > > ```
> > > I am inclined to changing the current behavior and reporting by pairs 
> > > because
> > > 1) this would simplify the code a lot.because we could basically reuse 
> > > the conflict detection from replacement library.
> > > 2) IMO, pairs are easier to read than grouping multiple conflicts - users 
> > > would still need to interpret the conflicts and figure out how all 
> > > replacements conflict in a group, while reporting as pairs already gives 
> > > you this information.
> > > 3) in practice, it's uncommon to see more than two conflicting 
> > > replacements at the same code location.
> > > 
> > > I would vote for the last approach (with 
> > > `llvm::toString(std::move(Err))`) which is easy to implement and doesn't 
> > > really regress the `UI` that much. WDYT?
> > I think if we can use `llvm::toString(std::move(Err))` for this patch, it 
> > will simplify the code and reuse already existing error message. Even if I 
> > found file+offset conflict location less readable than file+line+column.
> > 
> > I have made some prototype to "enhance" error reporting but I think they 
> > should be put in dedicated patches and need further discutions.
> > During my "research" I found we can use:
> > * Conflict marker format.
> > ```
> > /src/basic.h
> > @@ 12:23-12:23 @@
> > <<< Existing replacement
> >   virtual void func() noexcept {}
> > ===
> >   virtual void func() override {}
> > >>> New replacement
> > ```
> > * A unified diff like.
> > ```
> > /src/basic.h
> > @@ 12:23-12:23 @@
> > -   virtual void func() {}
> > +   virtual void func() noexcept {}
> > +   virtual void func() override {}
> > ```
> > * A clang like diagnostic message.
> > ```
> > /src/basic.h
> > @@ 12:23-12:23 @@
> >   virtual void func() {}
> >   ^
> >

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

2018-03-15 Thread Jeremy Demeule via Phabricator via cfe-commits

jdemeule added inline comments.



Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:353
+const FileEntry *Entry = FileAndReplacements.first;
+ReplacementsToAtomicChanges DeduplicatedChanges(SM, Entry);
+for (const auto  : FileAndReplacements.second)

ioeric wrote:
> jdemeule wrote:
> > ioeric wrote:
> > > Sorry that I didn't make myself clear... what you had in the previous 
> > > revision was correct (for the current use case of apply-all-replacements) 
> > > i.e. store all replacements in one `AtomicChange`, which allows you to 
> > > detect conflicts on the fly. So the code here can be simplified as:
> > > 
> > > ```
> > > ...
> > > Entry = ...;
> > > AtomicChange FileChange;
> > > for (const auto  : FileAndReplacements.second) {
> > >   auto Err = FileChange.replace(  );
> > >   if (Err)
> > > reportConflict(Entry, std::move(Err));  // reportConflict as we go
> > > }
> > > FileChanges.emplace(Entry, {FileChange});
> > > ...
> > > ```
> > > 
> > > I think with this set up, you wouldn't need 
> > > `ReplacementsToAtomicChanges`, `ConflictError` and `reportConflicts`.
> > I think I have over-interpreting your previous comment :-)
> > However, I am still confused about error reporting.
> > Currently, clang-apply-replacements reports conflicting replacement per 
> > file and per conflict.
> > For example:
> > ```
> > There are conflicting changes to XXX:
> > The following changes conflict:
> >   Replace 8:8-8:33 with "AAA"
> >   Replace 8:8-8:33 with "BBB"
> >   Remove 8:10-8:15
> >   Insert at 8:14 CCC
> > ```
> > 
> > With this patch, conflict are reported by pair (first 
> > replacement/conflicted one) and I am able to print the corresponding file 
> > only once (thanks to the defered reporting).
> > ```
> > There are conflicting changes to XXX:
> > The following changes conflict:
> >   Replace 8:8-8:33 with "AAA"
> >   Replace 8:8-8:33 with "BBB"
> > The following changes conflict:
> >   Replace 8:8-8:33 with "AAA"
> >   Remove 8:10-8:15
> > The following changes conflict:
> >   Replace 8:8-8:33 with "AAA"
> >   Insert at 8:14 CCC
> > ```
> > 
> > I prefer the way you suggest to report conflict but we will loose or print 
> > conflicting file at each conflict detected.
> > I even more prefer to use `llvm::toString(std::move(Err))` but this will 
> > fully change the reporting and will also be reported by pair.
> > ```
> > The new replacement overlaps with an existing replacement.
> > New replacement: XXX: 106:+26:"AAA"
> > Existing replacement: XXX: 106:+26:"BBB"
> > The new replacement overlaps with an existing replacement.
> > New replacement: XXX: 106:+26:"AAA"
> > Existing replacement: XXX: 112:+0:"CCC"
> > The new replacement overlaps with an existing replacement.
> > New replacement: XXX: 106:+26:"AAA"
> > Existing replacement: XXX: 108:+12:""
> > ```
> I am inclined to changing the current behavior and reporting by pairs because
> 1) this would simplify the code a lot.because we could basically reuse the 
> conflict detection from replacement library.
> 2) IMO, pairs are easier to read than grouping multiple conflicts - users 
> would still need to interpret the conflicts and figure out how all 
> replacements conflict in a group, while reporting as pairs already gives you 
> this information.
> 3) in practice, it's uncommon to see more than two conflicting replacements 
> at the same code location.
> 
> I would vote for the last approach (with `llvm::toString(std::move(Err))`) 
> which is easy to implement and doesn't really regress the `UI` that much. 
> WDYT?
I think if we can use `llvm::toString(std::move(Err))` for this patch, it will 
simplify the code and reuse already existing error message. Even if I found 
file+offset conflict location less readable than file+line+column.

I have made some prototype to "enhance" error reporting but I think they should 
be put in dedicated patches and need further discutions.
During my "research" I found we can use:
* Conflict marker format.
```
/src/basic.h
@@ 12:23-12:23 @@
<<< Existing replacement
  virtual void func() noexcept {}
===
  virtual void func() override {}
>>> New replacement
```
* A unified diff like.
```
/src/basic.h
@@ 12:23-12:23 @@
-   virtual void func() {}
+   virtual void func() noexcept {}
+   virtual void func() override {}
```
* A clang like diagnostic message.
```
/src/basic.h
@@ 12:23-12:23 @@
  virtual void func() {}
  ^
  noexcept 
  virtual void func() {}
  ^
  override 
```



https://reviews.llvm.org/D43764



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

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

2018-03-15 Thread Jeremy Demeule via Phabricator via cfe-commits

jdemeule updated this revision to Diff 138504.
jdemeule added a comment.

Update after review.
Add tests to check identical insertions and order dependent insertions.


https://reviews.llvm.org/D43764

Files:
  
clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h
  clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
  clang-apply-replacements/tool/CMakeLists.txt
  clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
  test/clang-apply-replacements/Inputs/basic/file2.yaml
  test/clang-apply-replacements/Inputs/conflict/expected.txt
  test/clang-apply-replacements/Inputs/identical/file1.yaml
  test/clang-apply-replacements/Inputs/identical/identical.cpp
  test/clang-apply-replacements/Inputs/order-dependent/expected.txt
  test/clang-apply-replacements/Inputs/order-dependent/file1.yaml
  test/clang-apply-replacements/Inputs/order-dependent/file2.yaml
  test/clang-apply-replacements/Inputs/order-dependent/order-dependent.cpp
  test/clang-apply-replacements/identical.cpp
  test/clang-apply-replacements/order-dependent.cpp
  unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
  unittests/clang-apply-replacements/CMakeLists.txt
  unittests/clang-apply-replacements/ReformattingTest.cpp

Index: unittests/clang-apply-replacements/ReformattingTest.cpp
===
--- unittests/clang-apply-replacements/ReformattingTest.cpp
+++ /dev/null
@@ -1,67 +0,0 @@
-//===- clang-apply-replacements/ReformattingTest.cpp --===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#include "clang-apply-replacements/Tooling/ApplyReplacements.h"
-#include "common/VirtualFileHelper.h"
-#include "clang/Format/Format.h"
-#include "clang/Tooling/Refactoring.h"
-#include "gtest/gtest.h"
-
-using namespace clang;
-using namespace clang::tooling;
-using namespace clang::replace;
-
-typedef std::vector ReplacementsVec;
-
-static Replacement makeReplacement(unsigned Offset, unsigned Length,
-   unsigned ReplacementLength,
-   llvm::StringRef FilePath = "") {
-  return Replacement(FilePath, Offset, Length,
- std::string(ReplacementLength, '~'));
-}
-
-// generate a set of replacements containing one element
-static ReplacementsVec makeReplacements(unsigned Offset, unsigned Length,
-unsigned ReplacementLength,
-llvm::StringRef FilePath = "~") {
-  ReplacementsVec Replaces;
-  Replaces.push_back(
-  makeReplacement(Offset, Length, ReplacementLength, FilePath));
-  return Replaces;
-}
-
-// Put these functions in the clang::tooling namespace so arg-dependent name
-// lookup finds these functions for the EXPECT_EQ macros below.
-namespace clang {
-namespace tooling {
-
-std::ostream <<(std::ostream , const Range ) {
-  return os << "Range(" << R.getOffset() << ", " << R.getLength() << ")";
-}
-
-} // namespace tooling
-} // namespace clang
-
-// Ensure zero-length ranges are produced. Even lines where things are deleted
-// need reformatting.
-TEST(CalculateChangedRangesTest, producesZeroLengthRange) {
-  RangeVector Changes = calculateChangedRanges(makeReplacements(0, 4, 0));
-  EXPECT_EQ(Range(0, 0), Changes.front());
-}
-
-// Basic test to ensure replacements turn into ranges properly.
-TEST(CalculateChangedRangesTest, calculatesRanges) {
-  ReplacementsVec R;
-  R.push_back(makeReplacement(2, 0, 3));
-  R.push_back(makeReplacement(5, 2, 4));
-  RangeVector Changes = calculateChangedRanges(R);
-
-  Range ExpectedRanges[] = { Range(2, 3), Range(8, 4) };
-  EXPECT_TRUE(std::equal(Changes.begin(), Changes.end(), ExpectedRanges));
-}
Index: unittests/clang-apply-replacements/CMakeLists.txt
===
--- unittests/clang-apply-replacements/CMakeLists.txt
+++ unittests/clang-apply-replacements/CMakeLists.txt
@@ -9,12 +9,12 @@
 
 add_extra_unittest(ClangApplyReplacementsTests
   ApplyReplacementsTest.cpp
-  ReformattingTest.cpp
   )
 
 target_link_libraries(ClangApplyReplacementsTests
   PRIVATE
   clangApplyReplacements
   clangBasic
   clangToolingCore
+  clangToolingRefactor
   )
Index: unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
===
--- unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
+++ unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
@@ -9,6 +9,7 @@
 //===--===//
 
 #include "clang-apply-replacements/Tooling/ApplyReplacements.h"
+#include "clang/Format/Format.h"
 #include "gtest/gtest.h"
 
 using namespace

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

2018-03-12 Thread Jeremy Demeule via Phabricator via cfe-commits

jdemeule added inline comments.



Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:213-221
   // Use the file manager to deduplicate paths. FileEntries are
   // automatically canonicalized.
-  if (const FileEntry *Entry = 
SM.getFileManager().getFile(R.getFilePath())) {
+  if (const FileEntry *Entry =
+  SM.getFileManager().getFile(R.getFilePath())) {
 GroupedReplacements[Entry].push_back(R);
   } else if (Warned.insert(R.getFilePath()).second) {
+errs() << "Described file '" << R.getFilePath()

ioeric wrote:
> This code block is the same as the one in the above loop. Consider pulling it 
> into a lambda.
Yes, for sure. I was not confident about this part.



Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:290
+std::vector Changes;
+for (const auto  : AllChanges.getReplacements()) {
+  tooling::AtomicChange Change(Entry->getName(), Entry->getName());

ioeric wrote:
> I might be missing something, but why do we need to pull replacements from 
> the result change into a set of changes? 
I interpret you suggestion of set of `AtomicChange` a little bit too much.



Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:353
+const FileEntry *Entry = FileAndReplacements.first;
+ReplacementsToAtomicChanges DeduplicatedChanges(SM, Entry);
+for (const auto  : FileAndReplacements.second)

ioeric wrote:
> Sorry that I didn't make myself clear... what you had in the previous 
> revision was correct (for the current use case of apply-all-replacements) 
> i.e. store all replacements in one `AtomicChange`, which allows you to detect 
> conflicts on the fly. So the code here can be simplified as:
> 
> ```
> ...
> Entry = ...;
> AtomicChange FileChange;
> for (const auto  : FileAndReplacements.second) {
>   auto Err = FileChange.replace(  );
>   if (Err)
> reportConflict(Entry, std::move(Err));  // reportConflict as we go
> }
> FileChanges.emplace(Entry, {FileChange});
> ...
> ```
> 
> I think with this set up, you wouldn't need `ReplacementsToAtomicChanges`, 
> `ConflictError` and `reportConflicts`.
I think I have over-interpreting your previous comment :-)
However, I am still confused about error reporting.
Currently, clang-apply-replacements reports conflicting replacement per file 
and per conflict.
For example:
```
There are conflicting changes to XXX:
The following changes conflict:
  Replace 8:8-8:33 with "AAA"
  Replace 8:8-8:33 with "BBB"
  Remove 8:10-8:15
  Insert at 8:14 CCC
```

With this patch, conflict are reported by pair (first replacement/conflicted 
one) and I am able to print the corresponding file only once (thanks to the 
defered reporting).
```
There are conflicting changes to XXX:
The following changes conflict:
  Replace 8:8-8:33 with "AAA"
  Replace 8:8-8:33 with "BBB"
The following changes conflict:
  Replace 8:8-8:33 with "AAA"
  Remove 8:10-8:15
The following changes conflict:
  Replace 8:8-8:33 with "AAA"
  Insert at 8:14 CCC
```

I prefer the way you suggest to report conflict but we will loose or print 
conflicting file at each conflict detected.
I even more prefer to use `llvm::toString(std::move(Err))` but this will fully 
change the reporting and will also be reported by pair.
```
The new replacement overlaps with an existing replacement.
New replacement: XXX: 106:+26:"AAA"
Existing replacement: XXX: 106:+26:"BBB"
The new replacement overlaps with an existing replacement.
New replacement: XXX: 106:+26:"AAA"
Existing replacement: XXX: 112:+0:"CCC"
The new replacement overlaps with an existing replacement.
New replacement: XXX: 106:+26:"AAA"
Existing replacement: XXX: 108:+12:""
```



Comment at: test/clang-apply-replacements/Inputs/basic/file2.yaml:1
 ---
 MainSourceFile: source2.cpp

ioeric wrote:
> Could you please add two test cases for insertions: 1) identical insertions 
> (e.g. "AB" and "AB") at the same location are applied sucessfully and 2) 
> order-dependent insertions (e.g. "AB" and "BA") are detected.
> 
> (It might make sense to do this in a different file)
I will.



Comment at: test/clang-apply-replacements/Inputs/conflict/expected.txt:8
+  Replace 9:5-9:11 with "elem"
 The following changes conflict:
   Remove 12:3-12:14

ioeric wrote:
> How could one replacement (`Remove 12:3-12:14`) conflict with itself?
It is not the case, they are reported by pair now.


https://reviews.llvm.org/D43764



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

[PATCH] D43500: [clang-tidy]: modernize-use-default-member-init: Remove trailing comma and colon.

2018-03-06 Thread Jeremy Demeule via Phabricator via cfe-commits

jdemeule added inline comments.



Comment at: unittests/clang-tidy/ClangTidyTest.h:145
+
+  if (Options.FormatStyle) {
+llvm::Expected Style = format::getStyle(

alexfh wrote:
> I wonder whether it's better to use lit for the tests that require formatting 
> than to expand clang-tidy unit test utilities with runCheckAndFormatOnCode? 
> What was the reason to use unit tests in this patch as opposed to lit tests?
Indeed, that a good question.
Personally, I found unit test easier to launch and debug than lit test and I 
notice that it is not possible to made a unit test which react the same way as 
clang-tidy.
When I wrote this test, I want it to react as in clang-tidy and for this I try 
to call `cleanupAroundReplacements` in the test.
Adding formatting was more a side effect to have some sustainable and 
comprehensive way to write test.



https://reviews.llvm.org/D43500



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

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

2018-03-04 Thread Jeremy Demeule via Phabricator via cfe-commits

jdemeule updated this revision to Diff 136944.
jdemeule added a comment.

Update patch after reviewer comments.


https://reviews.llvm.org/D43764

Files:
  
clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h
  clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
  clang-apply-replacements/tool/CMakeLists.txt
  clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
  test/clang-apply-replacements/Inputs/basic/basic.h
  test/clang-apply-replacements/Inputs/basic/file1.yaml
  test/clang-apply-replacements/Inputs/basic/file2.yaml
  test/clang-apply-replacements/Inputs/conflict/expected.txt
  unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
  unittests/clang-apply-replacements/CMakeLists.txt
  unittests/clang-apply-replacements/ReformattingTest.cpp

Index: unittests/clang-apply-replacements/ReformattingTest.cpp
===
--- unittests/clang-apply-replacements/ReformattingTest.cpp
+++ /dev/null
@@ -1,67 +0,0 @@
-//===- clang-apply-replacements/ReformattingTest.cpp --===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#include "clang-apply-replacements/Tooling/ApplyReplacements.h"
-#include "common/VirtualFileHelper.h"
-#include "clang/Format/Format.h"
-#include "clang/Tooling/Refactoring.h"
-#include "gtest/gtest.h"
-
-using namespace clang;
-using namespace clang::tooling;
-using namespace clang::replace;
-
-typedef std::vector ReplacementsVec;
-
-static Replacement makeReplacement(unsigned Offset, unsigned Length,
-   unsigned ReplacementLength,
-   llvm::StringRef FilePath = "") {
-  return Replacement(FilePath, Offset, Length,
- std::string(ReplacementLength, '~'));
-}
-
-// generate a set of replacements containing one element
-static ReplacementsVec makeReplacements(unsigned Offset, unsigned Length,
-unsigned ReplacementLength,
-llvm::StringRef FilePath = "~") {
-  ReplacementsVec Replaces;
-  Replaces.push_back(
-  makeReplacement(Offset, Length, ReplacementLength, FilePath));
-  return Replaces;
-}
-
-// Put these functions in the clang::tooling namespace so arg-dependent name
-// lookup finds these functions for the EXPECT_EQ macros below.
-namespace clang {
-namespace tooling {
-
-std::ostream <<(std::ostream , const Range ) {
-  return os << "Range(" << R.getOffset() << ", " << R.getLength() << ")";
-}
-
-} // namespace tooling
-} // namespace clang
-
-// Ensure zero-length ranges are produced. Even lines where things are deleted
-// need reformatting.
-TEST(CalculateChangedRangesTest, producesZeroLengthRange) {
-  RangeVector Changes = calculateChangedRanges(makeReplacements(0, 4, 0));
-  EXPECT_EQ(Range(0, 0), Changes.front());
-}
-
-// Basic test to ensure replacements turn into ranges properly.
-TEST(CalculateChangedRangesTest, calculatesRanges) {
-  ReplacementsVec R;
-  R.push_back(makeReplacement(2, 0, 3));
-  R.push_back(makeReplacement(5, 2, 4));
-  RangeVector Changes = calculateChangedRanges(R);
-
-  Range ExpectedRanges[] = { Range(2, 3), Range(8, 4) };
-  EXPECT_TRUE(std::equal(Changes.begin(), Changes.end(), ExpectedRanges));
-}
Index: unittests/clang-apply-replacements/CMakeLists.txt
===
--- unittests/clang-apply-replacements/CMakeLists.txt
+++ unittests/clang-apply-replacements/CMakeLists.txt
@@ -9,12 +9,12 @@
 
 add_extra_unittest(ClangApplyReplacementsTests
   ApplyReplacementsTest.cpp
-  ReformattingTest.cpp
   )
 
 target_link_libraries(ClangApplyReplacementsTests
   PRIVATE
   clangApplyReplacements
   clangBasic
   clangToolingCore
+  clangToolingRefactor
   )
Index: unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
===
--- unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
+++ unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
@@ -9,6 +9,7 @@
 //===--===//
 
 #include "clang-apply-replacements/Tooling/ApplyReplacements.h"
+#include "clang/Format/Format.h"
 #include "gtest/gtest.h"
 
 using namespace clang::replace;
@@ -41,11 +42,12 @@
   IntrusiveRefCntPtr(new DiagnosticIDs()), DiagOpts.get());
   FileManager Files((FileSystemOptions()));
   SourceManager SM(Diagnostics, Files);
+  TUReplacements TURs;
   TUDiagnostics TUs =
   makeTUDiagnostics("path/to/source.cpp", "diagnostic", {}, {}, "path/to");
-  FileToReplacementsMap ReplacementsMap;
+  FileToChangesMap ReplacementsMap;
 
-  EXPECT_TRUE(mergeAndDeduplicate(TUs,

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

2018-03-04 Thread Jeremy Demeule via Phabricator via cfe-commits

jdemeule added inline comments.



Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:207
+  llvm::DenseMap>
+  GroupedReplacements;
+

ioeric wrote:
> jdemeule wrote:
> > ioeric wrote:
> > > I don't think we need to do the deduplication here anymore. AtomicChange 
> > > handles duplicates for you. I think all you need to do here is to group 
> > > replacements by files and later convert replacements to atomic changes.
> > I think I wrongly use AtomicChange somewhere because it doesn't deduplicate 
> > same replacement automatically.
> > For exemple, in the test suite, basic test defines 2 time the same 
> > replacement (adding 'override ' at offset 148) and I do not manage to avoid 
> > AtomicChange to add 'override override '. This is why I have kept the 
> > deduplicate step.
> `AtomicChange` does deduplicate identical replacements but not insertions, 
> because it wouldn't know whether users really want the insertions to be 
> deduplicated or not (e.g. imagine a tool wants to insert two `)` at the same 
> location). So it doesn't support that test case intentionally. In general, 
> users (i.e. tools generating changes) are responsible for ensuring changes 
> are deduplicated/applied in the expected way by using the correct interface 
> (e.g. `replace`, `insert` etc).  I think it would probably make more sense to 
> change the test to deduplicate identical non-insertion replacements. It would 
> also make sense to add another test case where identical insertions are both 
> applied.
> 
> For more semantics of conflicting/duplicated replacements, see 
> https://github.com/llvm-mirror/clang/blob/master/include/clang/Tooling/Core/Replacement.h#L217
>  
That's make sense indeed. My confusion comes from I do not want to break 
existing tests. I will update them the way you describe.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43764



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

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

2018-02-28 Thread Jeremy Demeule via Phabricator via cfe-commits

jdemeule added a comment.

Thanks for your feedback, they are very precious to me!




Comment at: 
clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h:45
+/// \brief Map mapping file name to AtomicChange targeting that file.
+typedef llvm::DenseMap
+FileToChangeMap;

ioeric wrote:
> I would expect this to be a map from files to a group of AtomicChanges (e.g. 
> `std::vector`). In general, a single AtomicChange contains 
> changes around a single code location which are likely to conflict with each 
> other, and either all changes or no change is applied. A file usually 
> corresponds to a set of atomic changes. 
> 
> Intuitively, I think clang-apply-replacements should simple gather a set of 
> atomic changes for each file and let `applyAtomicChanges` handle the 
> conflicts, but its current behavior is to skip conflicting replacements and 
> keep applying other replacements. This is not ideal, but I guess I understand 
> where this came from, and we might not be able to fix this in this patch 
> since most tools produce replacements instead of AtomicChange.
> 
> I would still suggest make this a map something like `llvm::DenseMap clang::FileEntry *, std::vector>`. To preserve the 
> current behavior (i.e. skip conflicts), when you group all replacements for a 
> single file, you would still put all replacements into a single AtomicChange, 
> but when you actually put the change into the map, you put it as a vector of 
> a single change.
I got your point. I will update the patch this way.



Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:207
+  llvm::DenseMap>
+  GroupedReplacements;
+

ioeric wrote:
> I don't think we need to do the deduplication here anymore. AtomicChange 
> handles duplicates for you. I think all you need to do here is to group 
> replacements by files and later convert replacements to atomic changes.
I think I wrongly use AtomicChange somewhere because it doesn't deduplicate 
same replacement automatically.
For exemple, in the test suite, basic test defines 2 time the same replacement 
(adding 'override ' at offset 148) and I do not manage to avoid AtomicChange to 
add 'override override '. This is why I have kept the deduplicate step.



Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:279
+  if (!NewCode) {
+errs() << "Failed to apply fixes on " << File << "\n";
+return false;

ioeric wrote:
> You should handle the error in `llvm::Expected`. You could convert it to 
> string and add to the error message with 
> `llvm::toString(NewCode.takeError())`. It would be nice if we could have a 
> test case for such cases.
I will use `llvm::Expected` as you suggest.
Do you have some ideas to made a test failed at this level?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43764



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

[PATCH] D43500: [clang-tidy]: modernize-use-default-member-init: Remove trailing comma and colon.

2018-02-28 Thread Jeremy Demeule via Phabricator via cfe-commits

jdemeule updated this revision to Diff 136389.
jdemeule added a comment.

Update after review.


https://reviews.llvm.org/D43500

Files:
  unittests/clang-tidy/CMakeLists.txt
  unittests/clang-tidy/ClangTidyTest.h
  unittests/clang-tidy/ModernizerModuleTest.cpp

Index: unittests/clang-tidy/ModernizerModuleTest.cpp
===
--- /dev/null
+++ unittests/clang-tidy/ModernizerModuleTest.cpp
@@ -0,0 +1,92 @@
+#include "ClangTidyTest.h"
+#include "modernize/UseDefaultMemberInitCheck.h"
+#include "gtest/gtest.h"
+
+using namespace clang::tidy::modernize;
+
+namespace clang {
+namespace tidy {
+namespace test {
+
+TEST(UseDefaultMemberInitCheckTest, NoChanges) {
+  EXPECT_NO_CHANGES(
+  UseDefaultMemberInitCheck,
+  "struct S { S() = default; bool a_ = false; bool b_ = false; };");
+  EXPECT_NO_CHANGES(UseDefaultMemberInitCheck, "struct S { S() : a_(true), "
+   "b_(true) {} bool a_{false}; "
+   "bool b_{false}; };");
+}
+
+TEST(UseDefaultMemberInitCheckTest, Basic) {
+  EXPECT_EQ("struct S {\n"
+"  S() {}\n"
+"  bool a_{false};\n"
+"};",
+runCheckAndFormatOnCode(
+"struct S { S() : a_(false) {} bool a_; };"));
+}
+
+TEST(UseDefaultMemberInitCheckTest, SeveralInitializers) {
+  EXPECT_EQ(
+  "struct S {\n"
+  "  S() {}\n"
+  "  bool a_{false};\n"
+  "  bool b_{true};\n"
+  "};",
+  runCheckAndFormatOnCode(
+  "struct S { S() : a_(false), b_(true) {} bool a_; bool b_; };"));
+}
+
+TEST(UseDefaultMemberInitCheckTest, ExceptSpec) {
+  EXPECT_EQ(
+  "struct S {\n"
+  "  S() noexcept(true) {}\n"
+  "  bool a_{false};\n"
+  "  bool b_{true};\n"
+  "};",
+  runCheckAndFormatOnCode(
+  "struct S { S() noexcept(true) : a_(false), b_(true) {} bool a_; "
+  "bool b_; };"));
+  EXPECT_EQ(
+  "#define NOEXCEPT_(X) noexcept(X)\n"
+  "struct S {\n"
+  "  S() NOEXCEPT_(true) {}\n"
+  "  bool a_{false};\n"
+  "  bool b_{true};\n"
+  "};",
+  runCheckAndFormatOnCode(
+  "#define NOEXCEPT_(X) noexcept(X)\n"
+  "struct S { S() NOEXCEPT_(true) : a_(false), b_(true) {} bool a_; "
+  "bool b_; };"));
+}
+
+TEST(UseDefaultMemberInitCheckTest, OnExisting) {
+  EXPECT_EQ("struct S {\n"
+"  S() {}\n"
+"  bool a_{false};\n"
+"  bool b_{true};\n"
+"};",
+runCheckAndFormatOnCode(
+"struct S { S() : a_(false), b_(true) {} bool a_{false}; bool "
+"b_{true}; };"));
+  EXPECT_EQ("struct S {\n"
+"  S() : a_(true) {}\n"
+"  bool a_{false};\n"
+"  bool b_{true};\n"
+"};",
+runCheckAndFormatOnCode(
+"struct S { S() : a_(true), b_(true) {} bool a_{false}; bool "
+"b_{true}; };"));
+  EXPECT_EQ("struct S {\n"
+"  S() : b_(false) {}\n"
+"  bool a_{false};\n"
+"  bool b_{true};\n"
+"};",
+runCheckAndFormatOnCode(
+"struct S { S() : a_(false), b_(false) {} bool a_{false}; bool "
+"b_{true}; };"));
+}
+
+} // namespace test
+} // namespace tidy
+} // namespace clang
Index: unittests/clang-tidy/ClangTidyTest.h
===
--- unittests/clang-tidy/ClangTidyTest.h
+++ unittests/clang-tidy/ClangTidyTest.h
@@ -13,6 +13,7 @@
 #include "ClangTidy.h"
 #include "ClangTidyDiagnosticConsumer.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Format/Format.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Tooling/Refactoring.h"
@@ -140,15 +141,55 @@
   }
   if (Errors)
 *Errors = Context.getErrors();
-  auto Result = tooling::applyAllReplacements(Code, Fixes);
+
+  if (Options.FormatStyle) {
+llvm::Expected Style = format::getStyle(
+*Options.FormatStyle, Filename.getSingleStringRef(), "LLVM");
+
+llvm::Expected CleannedReplacements =
+format::cleanupAroundReplacements(Code, Fixes, *Style);
+if (!CleannedReplacements) {
+  llvm::errs() << llvm::toString(CleannedReplacements.takeError()) << "\n";
+  return "";
+}
+if (llvm::Expected FormattedReplacements =
+format::formatReplacements(Code, *CleannedReplacements, *Style)) {
+  CleannedReplacements = std::move(FormattedReplacements);
+  if (!CleannedReplacements)
+return "";
+} else {
+  llvm::errs() << llvm::toString(FormattedReplacements.takeError())
+   << ". Skipping formatting.\n";
+  return "";
+}
+Fixes = *CleannedReplacements;
+  }
+
+  llvm::Expected Result =
+  tooling::applyAllReplacements(Code, Fixes);
   if (!Result) {
 // FIXME: propogate the error.

[PATCH] D43500: [clang-tidy]: modernize-use-default-member-init: Remove trailing comma and colon.

2018-02-26 Thread Jeremy Demeule via Phabricator via cfe-commits

jdemeule added inline comments.



Comment at: unittests/clang-tidy/ClangTidyTest.h:159-160
+  CleannedReplacements = std::move(FormattedReplacements);
+  if (!CleannedReplacements)
+llvm_unreachable("!CleannedReplacements");
+} else {

aaron.ballman wrote:
> This smells like it should be an assert rather than an unreachable.
Is returning an empty string OK?
This should let the assertion on the test do the job.



Comment at: unittests/clang-tidy/ClangTidyTest.h:167
+if (!Result) {
+  // FIXME: propogate the error.
+  llvm::consumeError(Result.takeError());

aaron.ballman wrote:
> Are you intending to implement this fixme in this patch?
I suggest to remove the copy/paste containing the fixme. Is-it OK?
Maybe with some hints I can suggest a fix for this later.


https://reviews.llvm.org/D43500



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

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

2018-02-26 Thread Jeremy Demeule via Phabricator via cfe-commits

jdemeule created this revision.
jdemeule added reviewers: klimek, ioeric.
Herald added subscribers: cfe-commits, mgorny.

By converting Replacements by AtomicChange, clang-apply-replacements is able 
like clang-tidy to automatically cleanup and format changes.
This should permits to close this ticket: 
https://bugs.llvm.org/show_bug.cgi?id=35051 and attempt to follow hints from 
https://reviews.llvm.org/D43500 comments.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43764

Files:
  
clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h
  clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
  clang-apply-replacements/tool/CMakeLists.txt
  clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
  test/clang-apply-replacements/Inputs/conflict/expected.txt
  unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
  unittests/clang-apply-replacements/CMakeLists.txt
  unittests/clang-apply-replacements/ReformattingTest.cpp

Index: unittests/clang-apply-replacements/ReformattingTest.cpp
===
--- unittests/clang-apply-replacements/ReformattingTest.cpp
+++ /dev/null
@@ -1,67 +0,0 @@
-//===- clang-apply-replacements/ReformattingTest.cpp --===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#include "clang-apply-replacements/Tooling/ApplyReplacements.h"
-#include "common/VirtualFileHelper.h"
-#include "clang/Format/Format.h"
-#include "clang/Tooling/Refactoring.h"
-#include "gtest/gtest.h"
-
-using namespace clang;
-using namespace clang::tooling;
-using namespace clang::replace;
-
-typedef std::vector ReplacementsVec;
-
-static Replacement makeReplacement(unsigned Offset, unsigned Length,
-   unsigned ReplacementLength,
-   llvm::StringRef FilePath = "") {
-  return Replacement(FilePath, Offset, Length,
- std::string(ReplacementLength, '~'));
-}
-
-// generate a set of replacements containing one element
-static ReplacementsVec makeReplacements(unsigned Offset, unsigned Length,
-unsigned ReplacementLength,
-llvm::StringRef FilePath = "~") {
-  ReplacementsVec Replaces;
-  Replaces.push_back(
-  makeReplacement(Offset, Length, ReplacementLength, FilePath));
-  return Replaces;
-}
-
-// Put these functions in the clang::tooling namespace so arg-dependent name
-// lookup finds these functions for the EXPECT_EQ macros below.
-namespace clang {
-namespace tooling {
-
-std::ostream <<(std::ostream , const Range ) {
-  return os << "Range(" << R.getOffset() << ", " << R.getLength() << ")";
-}
-
-} // namespace tooling
-} // namespace clang
-
-// Ensure zero-length ranges are produced. Even lines where things are deleted
-// need reformatting.
-TEST(CalculateChangedRangesTest, producesZeroLengthRange) {
-  RangeVector Changes = calculateChangedRanges(makeReplacements(0, 4, 0));
-  EXPECT_EQ(Range(0, 0), Changes.front());
-}
-
-// Basic test to ensure replacements turn into ranges properly.
-TEST(CalculateChangedRangesTest, calculatesRanges) {
-  ReplacementsVec R;
-  R.push_back(makeReplacement(2, 0, 3));
-  R.push_back(makeReplacement(5, 2, 4));
-  RangeVector Changes = calculateChangedRanges(R);
-
-  Range ExpectedRanges[] = { Range(2, 3), Range(8, 4) };
-  EXPECT_TRUE(std::equal(Changes.begin(), Changes.end(), ExpectedRanges));
-}
Index: unittests/clang-apply-replacements/CMakeLists.txt
===
--- unittests/clang-apply-replacements/CMakeLists.txt
+++ unittests/clang-apply-replacements/CMakeLists.txt
@@ -9,12 +9,12 @@
 
 add_extra_unittest(ClangApplyReplacementsTests
   ApplyReplacementsTest.cpp
-  ReformattingTest.cpp
   )
 
 target_link_libraries(ClangApplyReplacementsTests
   PRIVATE
   clangApplyReplacements
   clangBasic
   clangToolingCore
+  clangToolingRefactor
   )
Index: unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
===
--- unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
+++ unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
@@ -41,11 +41,12 @@
   IntrusiveRefCntPtr(new DiagnosticIDs()), DiagOpts.get());
   FileManager Files((FileSystemOptions()));
   SourceManager SM(Diagnostics, Files);
+  TUReplacements TURs;
   TUDiagnostics TUs =
   makeTUDiagnostics("path/to/source.cpp", "diagnostic", {}, {}, "path/to");
-  FileToReplacementsMap ReplacementsMap;
+  FileToChangeMap ReplacementsMap;
 
-  EXPECT_TRUE(mergeAndDeduplicate(TUs, ReplacementsMap, SM));
+  EXPECT_TRUE(mergeAndDeduplicate(TURs, TUs, ReplacementsMap,

[PATCH] D43500: [clang-tidy]: modernize-use-default-member-init: Remove trailing comma and colon.

2018-02-26 Thread Jeremy Demeule via Phabricator via cfe-commits

jdemeule updated this revision to Diff 135895.
jdemeule added a comment.

As discuss, I have removed the modifications but keep the tests.
To have tests with meaning, I have also add a way to format code on 
'runCheckOnCode'.


https://reviews.llvm.org/D43500

Files:
  unittests/clang-tidy/CMakeLists.txt
  unittests/clang-tidy/ClangTidyTest.h
  unittests/clang-tidy/ModernizerModuleTest.cpp

Index: unittests/clang-tidy/ModernizerModuleTest.cpp
===
--- /dev/null
+++ unittests/clang-tidy/ModernizerModuleTest.cpp
@@ -0,0 +1,92 @@
+#include "ClangTidyTest.h"
+#include "modernize/UseDefaultMemberInitCheck.h"
+#include "gtest/gtest.h"
+
+using namespace clang::tidy::modernize;
+
+namespace clang {
+namespace tidy {
+namespace test {
+
+TEST(UseDefaultMemberInitCheckTest, NoChanges) {
+  EXPECT_NO_CHANGES(
+  UseDefaultMemberInitCheck,
+  "struct S { S() = default; bool a_ = false; bool b_ = false; };");
+  EXPECT_NO_CHANGES(UseDefaultMemberInitCheck, "struct S { S() : a_(true), "
+   "b_(true) {} bool a_{false}; "
+   "bool b_{false}; };");
+}
+
+TEST(UseDefaultMemberInitCheckTest, Basic) {
+  EXPECT_EQ("struct S {\n"
+"  S() {}\n"
+"  bool a_{false};\n"
+"};",
+runCheckAndFormatOnCode(
+"struct S { S() : a_(false) {} bool a_; };"));
+}
+
+TEST(UseDefaultMemberInitCheckTest, SeveralInitializers) {
+  EXPECT_EQ(
+  "struct S {\n"
+  "  S() {}\n"
+  "  bool a_{false};\n"
+  "  bool b_{true};\n"
+  "};",
+  runCheckAndFormatOnCode(
+  "struct S { S() : a_(false), b_(true) {} bool a_; bool b_; };"));
+}
+
+TEST(UseDefaultMemberInitCheckTest, ExceptSpec) {
+  EXPECT_EQ(
+  "struct S {\n"
+  "  S() noexcept(true) {}\n"
+  "  bool a_{false};\n"
+  "  bool b_{true};\n"
+  "};",
+  runCheckAndFormatOnCode(
+  "struct S { S() noexcept(true) : a_(false), b_(true) {} bool a_; "
+  "bool b_; };"));
+  EXPECT_EQ(
+  "#define NOEXCEPT_(X) noexcept(X)\n"
+  "struct S {\n"
+  "  S() NOEXCEPT_(true) {}\n"
+  "  bool a_{false};\n"
+  "  bool b_{true};\n"
+  "};",
+  runCheckAndFormatOnCode(
+  "#define NOEXCEPT_(X) noexcept(X)\n"
+  "struct S { S() NOEXCEPT_(true) : a_(false), b_(true) {} bool a_; "
+  "bool b_; };"));
+}
+
+TEST(UseDefaultMemberInitCheckTest, OnExisting) {
+  EXPECT_EQ("struct S {\n"
+"  S() {}\n"
+"  bool a_{false};\n"
+"  bool b_{true};\n"
+"};",
+runCheckAndFormatOnCode(
+"struct S { S() : a_(false), b_(true) {} bool a_{false}; bool "
+"b_{true}; };"));
+  EXPECT_EQ("struct S {\n"
+"  S() : a_(true) {}\n"
+"  bool a_{false};\n"
+"  bool b_{true};\n"
+"};",
+runCheckAndFormatOnCode(
+"struct S { S() : a_(true), b_(true) {} bool a_{false}; bool "
+"b_{true}; };"));
+  EXPECT_EQ("struct S {\n"
+"  S() : b_(false) {}\n"
+"  bool a_{false};\n"
+"  bool b_{true};\n"
+"};",
+runCheckAndFormatOnCode(
+"struct S { S() : a_(false), b_(false) {} bool a_{false}; bool "
+"b_{true}; };"));
+}
+
+} // namespace test
+} // namespace tidy
+} // namespace clang
Index: unittests/clang-tidy/ClangTidyTest.h
===
--- unittests/clang-tidy/ClangTidyTest.h
+++ unittests/clang-tidy/ClangTidyTest.h
@@ -13,6 +13,7 @@
 #include "ClangTidy.h"
 #include "ClangTidyDiagnosticConsumer.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Format/Format.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Tooling/Refactoring.h"
@@ -140,13 +141,60 @@
   }
   if (Errors)
 *Errors = Context.getErrors();
-  auto Result = tooling::applyAllReplacements(Code, Fixes);
-  if (!Result) {
-// FIXME: propogate the error.
-llvm::consumeError(Result.takeError());
-return "";
+
+  //   Options.
+  if (Options.FormatStyle) {
+auto Style = format::getStyle(*Options.FormatStyle,
+  Filename.getSingleStringRef(), "LLVM");
+
+llvm::Expected CleannedReplacements =
+format::cleanupAroundReplacements(Code, Fixes, *Style);
+if (!CleannedReplacements) {
+  llvm::errs() << llvm::toString(CleannedReplacements.takeError()) << "\n";
+  return "";
+}
+if (llvm::Expected FormattedReplacements =
+format::formatReplacements(Code, *CleannedReplacements, *Style)) {
+  CleannedReplacements = std::move(FormattedReplacements);
+  if (!CleannedReplacements)
+llvm_unreachable("!CleannedReplacements");
+} else {
+

[PATCH] D43500: [clang-tidy]: modernize-use-default-member-init: Remove trailing comma and colon.

2018-02-21 Thread Jeremy Demeule via Phabricator via cfe-commits

jdemeule added a comment.

In https://reviews.llvm.org/D43500#1013558, @malcolm.parsons wrote:

> In https://reviews.llvm.org/D43500#1013497, @aaron.ballman wrote:
>
> > Is there a way to make clang-apply-replacements smarter rather than forcing 
> > every check to jump through hoops? I'm worried that if we have to fix 
> > individual checks we'll just run into the same bug later.
>
>
> See 
> http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20161017/174238.html

I was not aware of //cleanupAroundReplacements//. It should be a better option 
than fixing every check one by one. I am working on adding it on 
clang-apply-replacement for now and another review will be propose soon.

Should we discard this patch or keep only the added tests (if you found them 
relevant after fixing the comments)?

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43500

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

[PATCH] D43500: modernize-use-default-member-init: Remove trailing comma and colon.

2018-02-19 Thread Jeremy Demeule via Phabricator via cfe-commits

jdemeule created this revision.
jdemeule added a reviewer: alexfh.
Herald added subscribers: cfe-commits, mgorny.

'modernize-user-default-member-init' does not automatically ask to remove comma 
and colon when replacements are produced.

It seems, when they are apply directly from clang-tidy, the RefactoringTool 
engine is smart enough to remove trailing tokens.
However, when fixes are exported, clang-apply-replacements cannot do the same 
trick and will lead trailing commas and colon as reported on 
https://bugs.llvm.org/show_bug.cgi?id=35051.

This patch made “modernize-use-default-member-init” generate explicitly the 
removal of these locations to made it work correctly in both case (-fix and 
-export-fixes).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43500

Files:
  clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
  clang-tidy/modernize/UseDefaultMemberInitCheck.h
  unittests/clang-tidy/CMakeLists.txt
  unittests/clang-tidy/ModernizerModuleTest.cpp

Index: unittests/clang-tidy/ModernizerModuleTest.cpp
===
--- /dev/null
+++ unittests/clang-tidy/ModernizerModuleTest.cpp
@@ -0,0 +1,64 @@
+#include "ClangTidyTest.h"
+#include "modernize/UseDefaultMemberInitCheck.h"
+#include "gtest/gtest.h"
+
+using namespace clang::tidy::modernize;
+
+namespace clang {
+namespace tidy {
+namespace test {
+
+TEST(UseDefaultMemberInitCheckTest, NoChanges) {
+  EXPECT_NO_CHANGES(
+  UseDefaultMemberInitCheck,
+  "struct S { S() = default; bool a_ = false; bool b_ = false; };");
+  EXPECT_NO_CHANGES(UseDefaultMemberInitCheck, "struct S { S() : a_(true), "
+   "b_(true) {} bool a_{false}; "
+   "bool b_{false}; };");
+}
+
+TEST(UseDefaultMemberInitCheckTest, Basic) {
+  EXPECT_EQ("struct S { S() {} bool a_{false}; };",
+runCheckOnCode(
+"struct S { S() : a_(false) {} bool a_; };"));
+}
+
+TEST(UseDefaultMemberInitCheckTest, SeveralInitializers) {
+  EXPECT_EQ(
+  "struct S { S() {} bool a_{false}; bool b_{true}; };",
+  runCheckOnCode(
+  "struct S { S() : a_(false), b_(true) {} bool a_; bool b_; };"));
+}
+
+TEST(UseDefaultMemberInitCheckTest, ExceptSpec) {
+  EXPECT_EQ(
+  "struct S { S() noexcept(true) {} bool a_{false}; bool b_{true}; };",
+  runCheckOnCode(
+  "struct S { S() noexcept(true) : a_(false), b_(true) {} bool a_; "
+  "bool b_; };"));
+  EXPECT_EQ(
+  "#define NOEXCEPT_(X) noexcept(X)\n"
+  "struct S { S() NOEXCEPT_(true) {} bool a_{false}; bool b_{true}; };",
+  runCheckOnCode(
+  "#define NOEXCEPT_(X) noexcept(X)\n"
+  "struct S { S() NOEXCEPT_(true) : a_(false), b_(true) {} bool a_; "
+  "bool b_; };"));
+}
+
+TEST(UseDefaultMemberInitCheckTest, OnExisting) {
+  EXPECT_EQ("struct S { S()  {} bool a_{false}; bool b_{true}; };",
+runCheckOnCode(
+"struct S { S() : a_(false), b_(true) {} bool a_{false}; bool "
+"b_{true}; };"));
+  EXPECT_EQ("struct S { S() : a_(true) {} bool a_{false}; bool b_{true}; };",
+runCheckOnCode(
+"struct S { S() : a_(true), b_(true) {} bool a_{false}; bool "
+"b_{true}; };"));
+  EXPECT_EQ("struct S { S() : b_(false) {} bool a_{false}; bool b_{true}; };",
+runCheckOnCode(
+"struct S { S() : a_(false), b_(false) {} bool a_{false}; bool "
+"b_{true}; };"));
+}
+} // namespace test
+} // namespace tidy
+} // namespace clang
Index: unittests/clang-tidy/CMakeLists.txt
===
--- unittests/clang-tidy/CMakeLists.txt
+++ unittests/clang-tidy/CMakeLists.txt
@@ -12,6 +12,7 @@
   IncludeInserterTest.cpp
   GoogleModuleTest.cpp
   LLVMModuleTest.cpp
+  ModernizerModuleTest.cpp
   NamespaceAliaserTest.cpp
   ObjCModuleTest.cpp
   OverlappingReplacementsTest.cpp
@@ -29,6 +30,7 @@
   clangTidyAndroidModule
   clangTidyGoogleModule
   clangTidyLLVMModule
+  clangTidyModernizeModule
   clangTidyObjCModule
   clangTidyReadabilityModule
   clangTidyUtils
Index: clang-tidy/modernize/UseDefaultMemberInitCheck.h
===
--- clang-tidy/modernize/UseDefaultMemberInitCheck.h
+++ clang-tidy/modernize/UseDefaultMemberInitCheck.h
@@ -31,12 +31,18 @@
 
 private:
   void checkDefaultInit(const ast_matchers::MatchFinder::MatchResult ,
-const CXXCtorInitializer *Init);
+const CXXCtorInitializer *Init,
+const CXXConstructorDecl *Ctor);
   void checkExistingInit(const ast_matchers::MatchFinder::MatchResult ,
- const CXXCtorInitializer *Init);
+ const CXXCtorInitializer *Init,
+ const CXXConstructorDecl *Ctor);
+
+  void

[PATCH] D69282: [RFC] Add a VCS conflict marker format printing on tooling::ReplacementError

[PATCH] D69475: [clang] Provide better fix-it on exception spec error

[PATCH] D69475: [clang] Provide better fix-it on exception spec error

[PATCH] D69282: [RFC] Add a VCS conflict marker format printing on tooling::ReplacementError

[PATCH] D69475: [clang] Provide better fix-it on exception spec error

[PATCH] D69282: [RFC] Add a VCS conflict marker format printing on tooling::ReplacementError

[PATCH] D69475: [clang] Provide better fix-it on exception spec error

[PATCH] D69282: [RFC] Add a VCS conflict marker format printing on tooling::ReplacementError

[PATCH] D43500: [clang-tidy]: modernize-use-default-member-init: Remove trailing comma and colon.

[PATCH] D43500: [clang-tidy]: modernize-use-default-member-init: Remove trailing comma and colon.

[PATCH] D48523: [clang-tidy] Update run-clang-tidy.py with vfsoverlay arg

[PATCH] D48523: [clang-tidy] Update run-clang-tidy.py with vfsoverlay arg

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

[PATCH] D43500: [clang-tidy]: modernize-use-default-member-init: Remove trailing comma and colon.

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

[PATCH] D43500: [clang-tidy]: modernize-use-default-member-init: Remove trailing comma and colon.

[PATCH] D43500: [clang-tidy]: modernize-use-default-member-init: Remove trailing comma and colon.

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

[PATCH] D43500: [clang-tidy]: modernize-use-default-member-init: Remove trailing comma and colon.

[PATCH] D43500: [clang-tidy]: modernize-use-default-member-init: Remove trailing comma and colon.

[PATCH] D43500: modernize-use-default-member-init: Remove trailing comma and colon.

34 matches

Site Navigation

Mail list logo

Footer information