[clang-tools-extra] r278099 - [clang-rename] fix bug with initializer lists

[Kirill Bobyrev via cfe-commits]

Author: omtcyfz
Date: Tue Aug  9 02:14:48 2016
New Revision: 278099

URL: http://llvm.org/viewvc/llvm-project?rev=278099&view=rev
Log:
[clang-rename] fix bug with initializer lists

Clang-rename is currently not able to find a symbol in initializer list. This
patch fixes described issue.

Reviewers: alexfh

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

Modified:
clang-tools-extra/trunk/clang-rename/USRFinder.cpp
clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp
clang-tools-extra/trunk/test/clang-rename/Field.cpp

Modified: clang-tools-extra/trunk/clang-rename/USRFinder.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-rename/USRFinder.cpp?rev=278099&r1=278098&r2=278099&view=diff
==
--- clang-tools-extra/trunk/clang-rename/USRFinder.cpp (original)
+++ clang-tools-extra/trunk/clang-rename/USRFinder.cpp Tue Aug  9 02:14:48 2016
@@ -90,6 +90,25 @@ public:
  TypeEndLoc);
   }
 
+  bool VisitCXXConstructorDecl(clang::CXXConstructorDecl *ConstructorDecl) {
+for (auto &Initializer : ConstructorDecl->inits()) {
+  if (Initializer->getSourceOrder() == -1) {
+// Ignore implicit initializers.
+continue;
+  }
+  if (const clang::FieldDecl *FieldDecl = Initializer->getMember()) {
+const SourceLocation InitBeginLoc = Initializer->getSourceLocation(),
+ InitEndLoc = Lexer::getLocForEndOfToken(
+ InitBeginLoc, 0, Context.getSourceManager(),
+ Context.getLangOpts());
+if (!setResult(FieldDecl, InitBeginLoc, InitEndLoc)) {
+  return false;
+}
+  }
+}
+return true;
+  }
+
   // Other:
 
   const NamedDecl *getNamedDecl() { return Result; }

Modified: clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp?rev=278099&r1=278098&r2=278099&view=diff
==
--- clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp (original)
+++ clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp Tue Aug  9 02:14:48 
2016
@@ -48,18 +48,9 @@ public:
 // Ignore implicit initializers.
 continue;
   }
-  if (const clang::FieldDecl *FieldDecl = Initializer->getAnyMember()) {
+  if (const clang::FieldDecl *FieldDecl = Initializer->getMember()) {
 if (USRSet.find(getUSRForDecl(FieldDecl)) != USRSet.end()) {
-  // The initializer refers to a field that is to be renamed.
-  SourceLocation Location = Initializer->getSourceLocation();
-  StringRef TokenName = Lexer::getSourceText(
-  CharSourceRange::getTokenRange(Location),
-  Context.getSourceManager(), Context.getLangOpts());
-  if (TokenName == PrevName) {
-// The token of the source location we find actually has the old
-// name.
-LocationsFound.push_back(Initializer->getSourceLocation());
-  }
+  LocationsFound.push_back(Initializer->getSourceLocation());
 }
   }
 }

Modified: clang-tools-extra/trunk/test/clang-rename/Field.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-rename/Field.cpp?rev=278099&r1=278098&r2=278099&view=diff
==
--- clang-tools-extra/trunk/test/clang-rename/Field.cpp (original)
+++ clang-tools-extra/trunk/test/clang-rename/Field.cpp Tue Aug  9 02:14:48 2016
@@ -1,14 +1,15 @@
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=148 -new-name=Bar %t.cpp -i --
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-
 class Baz {
-  int Foo;  // CHECK: Bar;
+  int Foo; /* Test 1 */ // CHECK: int Bar;
 public:
   Baz();
 };
 
-Baz::Baz() : Foo(0) {}  // CHECK: Baz::Baz() : Bar(0) {}
+Baz::Baz() : Foo(0) /* Test 2 */ {}  // CHECK: Baz::Baz() : Bar(0)
+
+// Test 1.
+// RUN: clang-rename -offset=18 -new-name=Bar %s -- | sed 's,//.*,,' | 
FileCheck %s
+// Test 2.
+// RUN: clang-rename -offset=89 -new-name=Bar %s -- | sed 's,//.*,,' | 
FileCheck %s
 
-// Use grep -FUbo 'Foo'  to get the correct offset of foo when changing
-// this file.
+// To find offsets after modifying the file, use:
+//   grep -Ubo 'Foo.*' 


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

Re: [PATCH] D23193: [clang-rename] fix bug with initializer lists

[Kirill Bobyrev via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL278099: [clang-rename] fix bug with initializer lists 
(authored by omtcyfz).

Changed prior to commit:
  https://reviews.llvm.org/D23193?vs=67289&id=67290#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D23193

Files:
  clang-tools-extra/trunk/clang-rename/USRFinder.cpp
  clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp
  clang-tools-extra/trunk/test/clang-rename/Field.cpp

Index: clang-tools-extra/trunk/clang-rename/USRFinder.cpp
===
--- clang-tools-extra/trunk/clang-rename/USRFinder.cpp
+++ clang-tools-extra/trunk/clang-rename/USRFinder.cpp
@@ -90,6 +90,25 @@
  TypeEndLoc);
   }
 
+  bool VisitCXXConstructorDecl(clang::CXXConstructorDecl *ConstructorDecl) {
+for (auto &Initializer : ConstructorDecl->inits()) {
+  if (Initializer->getSourceOrder() == -1) {
+// Ignore implicit initializers.
+continue;
+  }
+  if (const clang::FieldDecl *FieldDecl = Initializer->getMember()) {
+const SourceLocation InitBeginLoc = Initializer->getSourceLocation(),
+ InitEndLoc = Lexer::getLocForEndOfToken(
+ InitBeginLoc, 0, Context.getSourceManager(),
+ Context.getLangOpts());
+if (!setResult(FieldDecl, InitBeginLoc, InitEndLoc)) {
+  return false;
+}
+  }
+}
+return true;
+  }
+
   // Other:
 
   const NamedDecl *getNamedDecl() { return Result; }
Index: clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp
===
--- clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp
+++ clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp
@@ -48,18 +48,9 @@
 // Ignore implicit initializers.
 continue;
   }
-  if (const clang::FieldDecl *FieldDecl = Initializer->getAnyMember()) {
+  if (const clang::FieldDecl *FieldDecl = Initializer->getMember()) {
 if (USRSet.find(getUSRForDecl(FieldDecl)) != USRSet.end()) {
-  // The initializer refers to a field that is to be renamed.
-  SourceLocation Location = Initializer->getSourceLocation();
-  StringRef TokenName = Lexer::getSourceText(
-  CharSourceRange::getTokenRange(Location),
-  Context.getSourceManager(), Context.getLangOpts());
-  if (TokenName == PrevName) {
-// The token of the source location we find actually has the old
-// name.
-LocationsFound.push_back(Initializer->getSourceLocation());
-  }
+  LocationsFound.push_back(Initializer->getSourceLocation());
 }
   }
 }
Index: clang-tools-extra/trunk/test/clang-rename/Field.cpp
===
--- clang-tools-extra/trunk/test/clang-rename/Field.cpp
+++ clang-tools-extra/trunk/test/clang-rename/Field.cpp
@@ -1,14 +1,15 @@
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=148 -new-name=Bar %t.cpp -i --
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-
 class Baz {
-  int Foo;  // CHECK: Bar;
+  int Foo; /* Test 1 */ // CHECK: int Bar;
 public:
   Baz();
 };
 
-Baz::Baz() : Foo(0) {}  // CHECK: Baz::Baz() : Bar(0) {}
+Baz::Baz() : Foo(0) /* Test 2 */ {}  // CHECK: Baz::Baz() : Bar(0)
+
+// Test 1.
+// RUN: clang-rename -offset=18 -new-name=Bar %s -- | sed 's,//.*,,' | 
FileCheck %s
+// Test 2.
+// RUN: clang-rename -offset=89 -new-name=Bar %s -- | sed 's,//.*,,' | 
FileCheck %s
 
-// Use grep -FUbo 'Foo'  to get the correct offset of foo when changing
-// this file.
+// To find offsets after modifying the file, use:
+//   grep -Ubo 'Foo.*' 


Index: clang-tools-extra/trunk/clang-rename/USRFinder.cpp
===
--- clang-tools-extra/trunk/clang-rename/USRFinder.cpp
+++ clang-tools-extra/trunk/clang-rename/USRFinder.cpp
@@ -90,6 +90,25 @@
  TypeEndLoc);
   }
 
+  bool VisitCXXConstructorDecl(clang::CXXConstructorDecl *ConstructorDecl) {
+for (auto &Initializer : ConstructorDecl->inits()) {
+  if (Initializer->getSourceOrder() == -1) {
+// Ignore implicit initializers.
+continue;
+  }
+  if (const clang::FieldDecl *FieldDecl = Initializer->getMember()) {
+const SourceLocation InitBeginLoc = Initializer->getSourceLocation(),
+ InitEndLoc = Lexer::getLocForEndOfToken(
+ InitBeginLoc, 0, Context.getSourceManager(),
+ Context.getLangOpts());
+if (!setResult(FieldDecl, InitBeginLoc, InitEndLoc)) {
+  return false;
+}
+  }
+}
+return true;
+  }
+
   // Other:
 
   const NamedDecl *getNamedDecl() { return Result; }
Index: clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp

Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index

[Daniel Marjamäki via cfe-commits]

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

In https://reviews.llvm.org/D21134#508511, @aaron.ballman wrote:

> Is there a reason we don't want this check to be a part of the clang 
> frontend, rather than as a clang-tidy check?


I discussed this with frontend and clang-tidy people in the llvm meeting last 
year and we came to the conclusion it was better in clang-tidy.

I don't remember the exact reasons.

But I think that this code is just "weird". It could be a bug but I write the 
warning mostly for readability reasons.



Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:43
@@ +42,3 @@
+
+static StringRef getAsString(const MatchFinder::MatchResult &Result,
+ const Expr *E) {

danielmarjamaki wrote:
> alexfh wrote:
> > Looks like this repeats getText from clang/Tooling/FixIt.h.
> Yes indeed..
this is fixed. As you suggested I used createReplacement() in the fixit. It's 
much nicer!


https://reviews.llvm.org/D21134



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

Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index

[Daniel Marjamäki via cfe-commits]

danielmarjamaki added a comment.

In https://reviews.llvm.org/D21134#508524, @jroelofs wrote:

> I think the replacement is wrong for something like:
>
>   int *arr; int offs1, offs2;
>   offs1[arr + offs2] = 42;
>
>
> which I think would give:
>
>   int *arr; int offs1, offs2;
>   arr + offs2[offs1] = 42;
>
>
> If the precedence of the "indexing" argument is less than that of the 
> subscript operator, then you need to insert parens:
>
>   int *arr; int offs1, offs2;
>   (arr + offs2)[offs1] = 42;
>


I don't think so. The matcher says:

  hasRHS(ignoringParenImpCasts(
anyOf(stringLiteral(), declRefExpr(hasType(pointsTo(qualType(,
  memberExpr(hasType(pointsTo(qualType()))

I think it's hard to know what the replaced code should be. In some cases it 
might be better with:

  arr[offs1 + offs2] = 42;


https://reviews.llvm.org/D21134



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

Re: [PATCH] D23257: Fix clang-tidy crash when a single fix is applied on multiple files.

[Eric Liu via cfe-commits]

ioeric updated this revision to Diff 67292.
ioeric marked 4 inline comments as done.
ioeric added a comment.

- Nits fixed


https://reviews.llvm.org/D23257

Files:
  clang-tidy/ClangTidy.cpp
  clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tidy/ClangTidyDiagnosticConsumer.h
  test/clang-tidy/Inputs/modernize-pass-by-value/header-with-fix.h
  test/clang-tidy/modernize-pass-by-value-multi-fixes.cpp
  unittests/clang-tidy/ClangTidyTest.h

Index: unittests/clang-tidy/ClangTidyTest.h
===
--- unittests/clang-tidy/ClangTidyTest.h
+++ unittests/clang-tidy/ClangTidyTest.h
@@ -118,15 +118,18 @@
 
   DiagConsumer.finish();
   tooling::Replacements Fixes;
-  for (const ClangTidyError &Error : Context.getErrors())
-for (const auto &Fix : Error.Fix) {
-  auto Err = Fixes.add(Fix);
-  // FIXME: better error handling. Keep the behavior for now.
-  if (Err) {
-llvm::errs() << llvm::toString(std::move(Err)) << "\n";
-return "";
+  for (const ClangTidyError &Error : Context.getErrors()) {
+for (const auto &FileAndFixes : Error.Fix) {
+  for (const auto &Fix : FileAndFixes.second) {
+auto Err = Fixes.add(Fix);
+// FIXME: better error handling. Keep the behavior for now.
+if (Err) {
+  llvm::errs() << llvm::toString(std::move(Err)) << "\n";
+  return "";
+}
   }
 }
+  }
   if (Errors)
 *Errors = Context.getErrors();
   auto Result = tooling::applyAllReplacements(Code, Fixes);
Index: test/clang-tidy/modernize-pass-by-value-multi-fixes.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-pass-by-value-multi-fixes.cpp
@@ -0,0 +1,12 @@
+// RUN: cat %S/Inputs/modernize-pass-by-value/header-with-fix.h > %T/pass-by-value-header-with-fix.h
+// RUN: sed -e 's#//.*$##' %s > %t.cpp
+// RUN: clang-tidy %t.cpp -checks='-*,modernize-pass-by-value' -header-filter='.*' -fix -- -std=c++11 -I %T | FileCheck %s -check-prefix=CHECK-MESSAGES -implicit-check-not="{{warning|error}}:"
+// RUN: FileCheck -input-file=%t.cpp %s -check-prefix=CHECK-FIXES
+// RUN: FileCheck -input-file=%T/pass-by-value-header-with-fix.h %s -check-prefix=CHECK-HEADER-FIXES
+
+#include "pass-by-value-header-with-fix.h"
+// CHECK-HEADER-FIXES: Foo(S s);
+Foo::Foo(const S &s) : s(s) {}
+// CHECK-MESSAGES: :9:10: warning: pass by value and use std::move [modernize-pass-by-value]
+// CHECK-FIXES: #include 
+// CHECK-FIXES: Foo::Foo(S s) : s(std::move(s)) {}
Index: test/clang-tidy/Inputs/modernize-pass-by-value/header-with-fix.h
===
--- /dev/null
+++ test/clang-tidy/Inputs/modernize-pass-by-value/header-with-fix.h
@@ -0,0 +1,8 @@
+struct S {
+  S(S&&);
+  S(const S&);
+};
+struct Foo {
+  Foo(const S &s);
+  S s;
+};
Index: clang-tidy/ClangTidyDiagnosticConsumer.h
===
--- clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -62,7 +62,8 @@
 
   std::string CheckName;
   ClangTidyMessage Message;
-  tooling::Replacements Fix;
+  // Fixes grouped by file path.
+  llvm::StringMap Fix;
   SmallVector Notes;
 
   // A build directory of the diagnostic source file.
Index: clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -77,8 +77,8 @@
   assert(Range.getBegin().isFileID() && Range.getEnd().isFileID() &&
  "Only file locations supported in fix-it hints.");
 
-  auto Err =
-  Error.Fix.add(tooling::Replacement(SM, Range, FixIt.CodeToInsert));
+  tooling::Replacement Replacement(SM, Range, FixIt.CodeToInsert);
+  llvm::Error Err = Error.Fix[Replacement.getFilePath()].add(Replacement);
   // FIXME: better error handling.
   if (Err) {
 llvm::errs() << "Fix conflicts with existing fix! "
@@ -495,25 +495,29 @@
   std::vector Sizes;
   for (const auto &Error : Errors) {
 int Size = 0;
-for (const auto &Replace : Error.Fix)
-  Size += Replace.getLength();
+for (const auto &FileAndReplaces : Error.Fix) {
+  for (const auto &Replace : FileAndReplaces.second)
+Size += Replace.getLength();
+}
 Sizes.push_back(Size);
   }
 
   // Build events from error intervals.
   std::map> FileEvents;
   for (unsigned I = 0; I < Errors.size(); ++I) {
-for (const auto &Replace : Errors[I].Fix) {
-  unsigned Begin = Replace.getOffset();
-  unsigned End = Begin + Replace.getLength();
-  const std::string &FilePath = Replace.getFilePath();
-  // FIXME: Handle empty intervals, such as those from insertions.
-  if (Begin == End)
-continue;
-  FileEvents[FilePath].push_back(
-  Event(Begin, End, Event::ET_Begin, I, Sizes[I]));
-  FileEvents

[clang-tools-extra] r278101 - Fix clang-tidy crash when a single fix is applied on multiple files.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Tue Aug  9 02:54:49 2016
New Revision: 278101

URL: http://llvm.org/viewvc/llvm-project?rev=278101&view=rev
Log:
Fix clang-tidy crash when a single fix is applied on multiple files.

Summary:
tooling::Replacements only holds replacements for a single file, so
this patch makes Fix a map from file paths to tooling::Replacements so that it
can be applied on multiple files.

Reviewers: hokein, alexfh

Subscribers: Prazek, cfe-commits

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

Added:

clang-tools-extra/trunk/test/clang-tidy/Inputs/modernize-pass-by-value/header-with-fix.h

clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value-multi-fixes.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h
clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h

Modified: clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp?rev=278101&r1=278100&r2=278101&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp Tue Aug  9 02:54:49 2016
@@ -126,27 +126,32 @@ public:
   }
   auto Diag = Diags.Report(Loc, Diags.getCustomDiagID(Level, "%0 [%1]"))
   << Message.Message << Name;
-  for (const tooling::Replacement &Fix : Error.Fix) {
-// Retrieve the source range for applicable fixes. Macro definitions
-// on the command line have locations in a virtual buffer and don't
-// have valid file paths and are therefore not applicable.
-SourceRange Range;
-SourceLocation FixLoc;
-if (Fix.isApplicable()) {
-  SmallString<128> FixAbsoluteFilePath = Fix.getFilePath();
-  Files.makeAbsolutePath(FixAbsoluteFilePath);
-  FixLoc = getLocation(FixAbsoluteFilePath, Fix.getOffset());
-  SourceLocation FixEndLoc = FixLoc.getLocWithOffset(Fix.getLength());
-  Range = SourceRange(FixLoc, FixEndLoc);
-  Diag << FixItHint::CreateReplacement(Range, 
Fix.getReplacementText());
-}
-
-++TotalFixes;
-if (ApplyFixes) {
-  bool Success = Fix.isApplicable() && Fix.apply(Rewrite);
-  if (Success)
-++AppliedFixes;
-  FixLocations.push_back(std::make_pair(FixLoc, Success));
+  for (const auto &FileAndReplacements : Error.Fix) {
+for (const auto &Replacement : FileAndReplacements.second) {
+  // Retrieve the source range for applicable fixes. Macro definitions
+  // on the command line have locations in a virtual buffer and don't
+  // have valid file paths and are therefore not applicable.
+  SourceRange Range;
+  SourceLocation FixLoc;
+  if (Replacement.isApplicable()) {
+SmallString<128> FixAbsoluteFilePath = Replacement.getFilePath();
+Files.makeAbsolutePath(FixAbsoluteFilePath);
+FixLoc = getLocation(FixAbsoluteFilePath, Replacement.getOffset());
+SourceLocation FixEndLoc =
+FixLoc.getLocWithOffset(Replacement.getLength());
+Range = SourceRange(FixLoc, FixEndLoc);
+Diag << FixItHint::CreateReplacement(
+Range, Replacement.getReplacementText());
+  }
+
+  ++TotalFixes;
+  if (ApplyFixes) {
+bool Success =
+Replacement.isApplicable() && Replacement.apply(Rewrite);
+if (Success)
+  ++AppliedFixes;
+FixLocations.push_back(std::make_pair(FixLoc, Success));
+  }
 }
   }
 }
@@ -511,9 +516,12 @@ void handleErrors(const std::vector &Errors,
 raw_ostream &OS) {
   tooling::TranslationUnitReplacements TUR;
-  for (const ClangTidyError &Error : Errors)
-TUR.Replacements.insert(TUR.Replacements.end(), Error.Fix.begin(),
-Error.Fix.end());
+  for (const ClangTidyError &Error : Errors) {
+for (const auto &FileAndFixes : Error.Fix)
+  TUR.Replacements.insert(TUR.Replacements.end(),
+  FileAndFixes.second.begin(),
+  FileAndFixes.second.end());
+  }
 
   yaml::Output YAML(OS);
   YAML << TUR;

Modified: clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp?rev=278101&r1=278100&r2=278101&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp Tue Aug  
9 02:54:49 20

Re: [PATCH] D23257: Fix clang-tidy crash when a single fix is applied on multiple files.

[Eric Liu via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL278101: Fix clang-tidy crash when a single fix is applied on 
multiple files. (authored by ioeric).

Changed prior to commit:
  https://reviews.llvm.org/D23257?vs=67292&id=67293#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D23257

Files:
  clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
  clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h
  
clang-tools-extra/trunk/test/clang-tidy/Inputs/modernize-pass-by-value/header-with-fix.h
  
clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value-multi-fixes.cpp
  clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h

Index: clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value-multi-fixes.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value-multi-fixes.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value-multi-fixes.cpp
@@ -0,0 +1,12 @@
+// RUN: cat %S/Inputs/modernize-pass-by-value/header-with-fix.h > %T/pass-by-value-header-with-fix.h
+// RUN: sed -e 's#//.*$##' %s > %t.cpp
+// RUN: clang-tidy %t.cpp -checks='-*,modernize-pass-by-value' -header-filter='.*' -fix -- -std=c++11 -I %T | FileCheck %s -check-prefix=CHECK-MESSAGES -implicit-check-not="{{warning|error}}:"
+// RUN: FileCheck -input-file=%t.cpp %s -check-prefix=CHECK-FIXES
+// RUN: FileCheck -input-file=%T/pass-by-value-header-with-fix.h %s -check-prefix=CHECK-HEADER-FIXES
+
+#include "pass-by-value-header-with-fix.h"
+// CHECK-HEADER-FIXES: Foo(S s);
+Foo::Foo(const S &s) : s(s) {}
+// CHECK-MESSAGES: :9:10: warning: pass by value and use std::move [modernize-pass-by-value]
+// CHECK-FIXES: #include 
+// CHECK-FIXES: Foo::Foo(S s) : s(std::move(s)) {}
Index: clang-tools-extra/trunk/test/clang-tidy/Inputs/modernize-pass-by-value/header-with-fix.h
===
--- clang-tools-extra/trunk/test/clang-tidy/Inputs/modernize-pass-by-value/header-with-fix.h
+++ clang-tools-extra/trunk/test/clang-tidy/Inputs/modernize-pass-by-value/header-with-fix.h
@@ -0,0 +1,8 @@
+struct S {
+  S(S&&);
+  S(const S&);
+};
+struct Foo {
+  Foo(const S &s);
+  S s;
+};
Index: clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h
===
--- clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h
+++ clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h
@@ -118,15 +118,18 @@
 
   DiagConsumer.finish();
   tooling::Replacements Fixes;
-  for (const ClangTidyError &Error : Context.getErrors())
-for (const auto &Fix : Error.Fix) {
-  auto Err = Fixes.add(Fix);
-  // FIXME: better error handling. Keep the behavior for now.
-  if (Err) {
-llvm::errs() << llvm::toString(std::move(Err)) << "\n";
-return "";
+  for (const ClangTidyError &Error : Context.getErrors()) {
+for (const auto &FileAndFixes : Error.Fix) {
+  for (const auto &Fix : FileAndFixes.second) {
+auto Err = Fixes.add(Fix);
+// FIXME: better error handling. Keep the behavior for now.
+if (Err) {
+  llvm::errs() << llvm::toString(std::move(Err)) << "\n";
+  return "";
+}
   }
 }
+  }
   if (Errors)
 *Errors = Context.getErrors();
   auto Result = tooling::applyAllReplacements(Code, Fixes);
Index: clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
===
--- clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
+++ clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
@@ -126,27 +126,32 @@
   }
   auto Diag = Diags.Report(Loc, Diags.getCustomDiagID(Level, "%0 [%1]"))
   << Message.Message << Name;
-  for (const tooling::Replacement &Fix : Error.Fix) {
-// Retrieve the source range for applicable fixes. Macro definitions
-// on the command line have locations in a virtual buffer and don't
-// have valid file paths and are therefore not applicable.
-SourceRange Range;
-SourceLocation FixLoc;
-if (Fix.isApplicable()) {
-  SmallString<128> FixAbsoluteFilePath = Fix.getFilePath();
-  Files.makeAbsolutePath(FixAbsoluteFilePath);
-  FixLoc = getLocation(FixAbsoluteFilePath, Fix.getOffset());
-  SourceLocation FixEndLoc = FixLoc.getLocWithOffset(Fix.getLength());
-  Range = SourceRange(FixLoc, FixEndLoc);
-  Diag << FixItHint::CreateReplacement(Range, Fix.getReplacementText());
-}
-
-++TotalFixes;
-if (ApplyFixes) {
-  bool Success = Fix.isApplicable() && Fix.apply(Rewrite);
-  if (Success)
-++AppliedFixes;
-  FixLocations.push_back(std::make_pair(FixLoc, Success));
+  for (const auto &FileAndReplacements : Error

Re: [PATCH] D23266: [include-fixer] Support processing multiple files in one run.

[Haojian Wu via cfe-commits]

hokein updated this revision to Diff 67294.
hokein added a comment.

Update comments, don't mention absolute file path since this is no guarantee
about that.


https://reviews.llvm.org/D23266

Files:
  include-fixer/IncludeFixer.cpp
  include-fixer/IncludeFixer.h
  include-fixer/IncludeFixerContext.cpp
  include-fixer/IncludeFixerContext.h
  include-fixer/tool/ClangIncludeFixer.cpp
  include-fixer/tool/clang-include-fixer.py
  test/include-fixer/commandline_options.cpp
  test/include-fixer/multiple_fixes.cpp
  unittests/include-fixer/IncludeFixerTest.cpp

Index: unittests/include-fixer/IncludeFixerTest.cpp
===
--- unittests/include-fixer/IncludeFixerTest.cpp
+++ unittests/include-fixer/IncludeFixerTest.cpp
@@ -88,15 +88,15 @@
   SymbolIndexMgr->addSymbolIndex(
   llvm::make_unique(Symbols));
 
-  IncludeFixerContext FixerContext;
-  IncludeFixerActionFactory Factory(*SymbolIndexMgr, FixerContext, "llvm");
-
+  std::vector FixerContexts;
+  IncludeFixerActionFactory Factory(*SymbolIndexMgr, FixerContexts, "llvm");
   std::string FakeFileName = "input.cc";
   runOnCode(&Factory, Code, FakeFileName, ExtraArgs);
-  if (FixerContext.getHeaderInfos().empty())
+  assert(FixerContexts.size() == 1);
+  if (FixerContexts.front().getHeaderInfos().empty())
 return Code;
   auto Replaces = clang::include_fixer::createIncludeFixerReplacements(
-  Code, FakeFileName, FixerContext);
+  Code, FixerContexts.front());
   EXPECT_TRUE(static_cast(Replaces))
   << llvm::toString(Replaces.takeError()) << "\n";
   if (!Replaces)
Index: test/include-fixer/multiple_fixes.cpp
===
--- /dev/null
+++ test/include-fixer/multiple_fixes.cpp
@@ -0,0 +1,13 @@
+// REQUIRES: shell
+// RUN: sed -e 's#//.*$##' %s > %t.cpp
+// RUN: mkdir -p %T/include-fixer/multiple-fixes
+// RUN: echo 'foo f;' > %T/include-fixer/multiple-fixes/foo.cpp
+// RUN: echo 'bar b;' > %T/include-fixer/multiple-fixes/bar.cpp
+// RUN: clang-include-fixer -db=fixed -input='foo= "foo.h";bar= "bar.h"' %T/include-fixer/multiple-fixes/*.cpp --
+// RUN: FileCheck -input-file=%T/include-fixer/multiple-fixes/bar.cpp %s -check-prefix=CHECK-BAR
+// RUN: FileCheck -input-file=%T/include-fixer/multiple-fixes/foo.cpp %s -check-prefix=CHECK-FOO
+//
+// CHECK-FOO: #include "foo.h"
+// CHECK-FOO: foo f;
+// CHECK-BAR: #include "bar.h"
+// CHECK-BAR: bar b;
Index: test/include-fixer/commandline_options.cpp
===
--- test/include-fixer/commandline_options.cpp
+++ test/include-fixer/commandline_options.cpp
@@ -1,9 +1,9 @@
 // REQUIRES: shell
 // RUN: echo "foo f;" > %t.cpp
 // RUN: clang-include-fixer -db=fixed -input='foo= "foo.h","bar.h"' -output-headers %t.cpp -- | FileCheck %s
-// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"}]}' %t.cpp | FileCheck %s -check-prefix=CHECK-CODE
-// RUN: cat %t.cpp | not clang-include-fixer -stdin -insert-header='{QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"},{Header: "\"foo2.h\"", QualifiedName: "foo"}]}' %t.cpp
-// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "a:foo"},{Header: "\"foo.h\"", QualifiedName: "b:foo"}]}' %t.cpp
+// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{FilePath: %t.cpp, QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"}]}' %t.cpp | FileCheck %s -check-prefix=CHECK-CODE
+// RUN: cat %t.cpp | not clang-include-fixer -stdin -insert-header='{FilePath: %t.cpp, QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"},{Header: "\"foo2.h\"", QualifiedName: "foo"}]}' %t.cpp
+// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{FilePath: %t.cpp, QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "a:foo"},{Header: "\"foo.h\"", QualifiedName: "b:foo"}]}' %t.cpp
 //
 // CHECK: "HeaderInfos": [
 // CHECK-NEXT:  {"Header": "\"foo.h\"",
Index: include-fixer/tool/clang-include-fixer.py
===
--- include-fixer/tool/clang-include-fixer.py
+++ include-fixer/tool/clang-include-fixer.py
@@ -149,21 +149,16 @@
 return
 
   try:
-# If there is only one suggested header, insert it directly.
-if len(unique_headers) == 1 or maximum_suggested_headers == 1:
-  InsertHeaderToVimBuffer({"QuerySymbolInfos": query_symbol_infos,
-   "Header

Re: [libcxxabi] r278058 - Do not depend on unwind when building standalone

[Asiri Rathnayake via cfe-commits]

He Petr,

This commit (and #121757) have broken a couple of builders, see:
http://lab.llvm.org:8011/builders/libcxx-libcxxabi-libunwind-x86_64-linux-debian/builds/135

We are also seeing this on our downstream builders.

Cheers,

/ Asiri


On Mon, Aug 8, 2016 at 11:09 PM, Petr Hosek via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: phosek
> Date: Mon Aug  8 17:09:54 2016
> New Revision: 278058
>
> URL: http://llvm.org/viewvc/llvm-project?rev=278058&view=rev
> Log:
> Do not depend on unwind when building standalone
>
> When libcxxabi is being built standalone, unwind dependency is not
> available, so do not use it even when LLVM unwinder is being
> requested.
>
> Differential Revision: https://reviews.llvm.org/D23228
>
> Modified:
> libcxxabi/trunk/test/CMakeLists.txt
>
> Modified: libcxxabi/trunk/test/CMakeLists.txt
> URL: http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/test/
> CMakeLists.txt?rev=278058&r1=278057&r2=278058&view=diff
> 
> ==
> --- libcxxabi/trunk/test/CMakeLists.txt (original)
> +++ libcxxabi/trunk/test/CMakeLists.txt Mon Aug  8 17:09:54 2016
> @@ -36,10 +36,9 @@ endif()
>
>  if (NOT LIBCXXABI_BUILT_STANDALONE)
>list(APPEND LIBCXXABI_TEST_DEPS cxx)
> -endif()
> -
> -if (LIBCXXABI_USE_LLVM_UNWINDER)
> -  list(APPEND LIBCXXABI_TEST_DEPS unwind)
> +  if (LIBCXXABI_USE_LLVM_UNWINDER)
> +list(APPEND LIBCXXABI_TEST_DEPS unwind)
> +  endif()
>  endif()
>
>  add_lit_testsuite(check-libcxxabi "Running libcxxabi tests"
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r278102 - [include-fixer] Support processing multiple files in one run.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Tue Aug  9 03:26:19 2016
New Revision: 278102

URL: http://llvm.org/viewvc/llvm-project?rev=278102&view=rev
Log:
[include-fixer] Support processing multiple files in one run.

Summary:
Previously, if we pass multiple files or a file pattern (e.g. /path/to/*.cc) to
include-fixer, include-fixer will apply all replacements to the first argument,
which probably causes crashes.

With this patch, include-fixer can process multiple files now.

Vim and Emacs integration are tested manually.

Reviewers: bkramer

Subscribers: cfe-commits

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

Added:
clang-tools-extra/trunk/test/include-fixer/multiple_fixes.cpp
Modified:
clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
clang-tools-extra/trunk/include-fixer/IncludeFixer.h
clang-tools-extra/trunk/include-fixer/IncludeFixerContext.cpp
clang-tools-extra/trunk/include-fixer/IncludeFixerContext.h
clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp
clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.py
clang-tools-extra/trunk/test/include-fixer/commandline_options.cpp
clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp

Modified: clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp?rev=278102&r1=278101&r2=278102&view=diff
==
--- clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp (original)
+++ clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp Tue Aug  9 03:26:19 
2016
@@ -37,6 +37,7 @@ public:
   std::unique_ptr
   CreateASTConsumer(clang::CompilerInstance &Compiler,
 StringRef InFile) override {
+FilePath = InFile;
 return llvm::make_unique();
   }
 
@@ -228,7 +229,7 @@ public:
 Symbol.getContexts(),
 Symbol.getNumOccurrences());
 }
-return IncludeFixerContext(QuerySymbolInfos, SymbolCandidates);
+return IncludeFixerContext(FilePath, QuerySymbolInfos, SymbolCandidates);
   }
 
 private:
@@ -297,6 +298,9 @@ private:
   /// recovery.
   std::vector MatchedSymbols;
 
+  /// The file path to the file being processed.
+  std::string FilePath;
+
   /// Whether we should use the smallest possible include path.
   bool MinimizeIncludePaths = true;
 };
@@ -304,9 +308,10 @@ private:
 } // namespace
 
 IncludeFixerActionFactory::IncludeFixerActionFactory(
-SymbolIndexManager &SymbolIndexMgr, IncludeFixerContext &Context,
-StringRef StyleName, bool MinimizeIncludePaths)
-: SymbolIndexMgr(SymbolIndexMgr), Context(Context),
+SymbolIndexManager &SymbolIndexMgr,
+std::vector &Contexts, StringRef StyleName,
+bool MinimizeIncludePaths)
+: SymbolIndexMgr(SymbolIndexMgr), Contexts(Contexts),
   MinimizeIncludePaths(MinimizeIncludePaths) {}
 
 IncludeFixerActionFactory::~IncludeFixerActionFactory() = default;
@@ -337,9 +342,9 @@ bool IncludeFixerActionFactory::runInvoc
   llvm::make_unique(SymbolIndexMgr, MinimizeIncludePaths);
   Compiler.ExecuteAction(*ScopedToolAction);
 
-  Context = ScopedToolAction->getIncludeFixerContext(
+  Contexts.push_back(ScopedToolAction->getIncludeFixerContext(
   Compiler.getSourceManager(),
-  Compiler.getPreprocessor().getHeaderSearchInfo());
+  Compiler.getPreprocessor().getHeaderSearchInfo()));
 
   // Technically this should only return true if we're sure that we have a
   // parseable file. We don't know that though. Only inform users of fatal
@@ -348,10 +353,11 @@ bool IncludeFixerActionFactory::runInvoc
 }
 
 llvm::Expected createIncludeFixerReplacements(
-StringRef Code, StringRef FilePath, const IncludeFixerContext &Context,
+StringRef Code, const IncludeFixerContext &Context,
 const clang::format::FormatStyle &Style, bool AddQualifiers) {
   if (Context.getHeaderInfos().empty())
 return tooling::Replacements();
+  StringRef FilePath = Context.getFilePath();
   std::string IncludeName =
   "#include " + Context.getHeaderInfos().front().Header + "\n";
   // Create replacements for the new header.

Modified: clang-tools-extra/trunk/include-fixer/IncludeFixer.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/IncludeFixer.h?rev=278102&r1=278101&r2=278102&view=diff
==
--- clang-tools-extra/trunk/include-fixer/IncludeFixer.h (original)
+++ clang-tools-extra/trunk/include-fixer/IncludeFixer.h Tue Aug  9 03:26:19 
2016
@@ -34,7 +34,8 @@ public:
   /// \param StyleName Fallback style for reformatting.
   /// \param MinimizeIncludePaths whether inserted include paths are optimized.
   IncludeFixerActionFactory(SymbolIndexManager &SymbolIndexMgr,
-IncludeFixerContext &Context, StringRef StyleName,
+ 

Re: [PATCH] D23266: [include-fixer] Support processing multiple files in one run.

[Haojian Wu via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL278102: [include-fixer] Support processing multiple files in 
one run. (authored by hokein).

Changed prior to commit:
  https://reviews.llvm.org/D23266?vs=67294&id=67296#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D23266

Files:
  clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
  clang-tools-extra/trunk/include-fixer/IncludeFixer.h
  clang-tools-extra/trunk/include-fixer/IncludeFixerContext.cpp
  clang-tools-extra/trunk/include-fixer/IncludeFixerContext.h
  clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp
  clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.py
  clang-tools-extra/trunk/test/include-fixer/commandline_options.cpp
  clang-tools-extra/trunk/test/include-fixer/multiple_fixes.cpp
  clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp

Index: clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp
===
--- clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp
+++ clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp
@@ -88,15 +88,15 @@
   SymbolIndexMgr->addSymbolIndex(
   llvm::make_unique(Symbols));
 
-  IncludeFixerContext FixerContext;
-  IncludeFixerActionFactory Factory(*SymbolIndexMgr, FixerContext, "llvm");
-
+  std::vector FixerContexts;
+  IncludeFixerActionFactory Factory(*SymbolIndexMgr, FixerContexts, "llvm");
   std::string FakeFileName = "input.cc";
   runOnCode(&Factory, Code, FakeFileName, ExtraArgs);
-  if (FixerContext.getHeaderInfos().empty())
+  assert(FixerContexts.size() == 1);
+  if (FixerContexts.front().getHeaderInfos().empty())
 return Code;
   auto Replaces = clang::include_fixer::createIncludeFixerReplacements(
-  Code, FakeFileName, FixerContext);
+  Code, FixerContexts.front());
   EXPECT_TRUE(static_cast(Replaces))
   << llvm::toString(Replaces.takeError()) << "\n";
   if (!Replaces)
Index: clang-tools-extra/trunk/include-fixer/IncludeFixerContext.cpp
===
--- clang-tools-extra/trunk/include-fixer/IncludeFixerContext.cpp
+++ clang-tools-extra/trunk/include-fixer/IncludeFixerContext.cpp
@@ -76,9 +76,9 @@
 } // anonymous namespace
 
 IncludeFixerContext::IncludeFixerContext(
-std::vector QuerySymbols,
+StringRef FilePath, std::vector QuerySymbols,
 std::vector Symbols)
-: QuerySymbolInfos(std::move(QuerySymbols)),
+: FilePath(FilePath), QuerySymbolInfos(std::move(QuerySymbols)),
   MatchedSymbols(std::move(Symbols)) {
   // Remove replicated QuerySymbolInfos with the same range.
   //
Index: clang-tools-extra/trunk/include-fixer/IncludeFixer.h
===
--- clang-tools-extra/trunk/include-fixer/IncludeFixer.h
+++ clang-tools-extra/trunk/include-fixer/IncludeFixer.h
@@ -34,7 +34,8 @@
   /// \param StyleName Fallback style for reformatting.
   /// \param MinimizeIncludePaths whether inserted include paths are optimized.
   IncludeFixerActionFactory(SymbolIndexManager &SymbolIndexMgr,
-IncludeFixerContext &Context, StringRef StyleName,
+std::vector &Contexts,
+StringRef StyleName,
 bool MinimizeIncludePaths = true);
 
   ~IncludeFixerActionFactory() override;
@@ -49,8 +50,8 @@
   /// The client to use to find cross-references.
   SymbolIndexManager &SymbolIndexMgr;
 
-  /// The context that contains all information about the symbol being queried.
-  IncludeFixerContext &Context;
+  /// Multiple contexts for files being processed.
+  std::vector &Contexts;
 
   /// Whether inserted include paths should be optimized.
   bool MinimizeIncludePaths;
@@ -65,7 +66,6 @@
 /// first header for insertion.
 ///
 /// \param Code The source code.
-/// \param FilePath The source file path.
 /// \param Context The context which contains all information for creating
 /// include-fixer replacements.
 /// \param Style clang-format style being used.
@@ -76,7 +76,7 @@
 /// qualifiers on success; otherwise, an llvm::Error carrying llvm::StringError
 /// is returned.
 llvm::Expected createIncludeFixerReplacements(
-StringRef Code, StringRef FilePath, const IncludeFixerContext &Context,
+StringRef Code, const IncludeFixerContext &Context,
 const format::FormatStyle &Style = format::getLLVMStyle(),
 bool AddQualifiers = true);
 
Index: clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
===
--- clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
+++ clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
@@ -37,6 +37,7 @@
   std::unique_ptr
   CreateASTConsumer(clang::CompilerInstance &Compiler,
 StringRef InFile) override {
+FilePath = InFile;
 return llvm

[PATCH] D23298: [clang-rename] nit: use isWritten

[Alexander Shaposhnikov via cfe-commits]

alexshap created this revision.
alexshap added reviewers: omtcyfz, alexfh.
alexshap added a subscriber: cfe-commits.
alexshap changed the visibility of this Differential Revision from "Public (No 
Login Required)" to "All Users".

nit: use isWritten and const auto *Initializer in 
NamedDeclFindingASTVisitor::VisitCXXConstructorDecl method.
Test plan: make -j8 check-clang-tools (passed)

https://reviews.llvm.org/D23298

Files:
  clang-rename/USRFinder.cpp

Index: clang-rename/USRFinder.cpp
===
--- clang-rename/USRFinder.cpp
+++ clang-rename/USRFinder.cpp
@@ -91,8 +91,8 @@
   }
 
   bool VisitCXXConstructorDecl(clang::CXXConstructorDecl *ConstructorDecl) {
-for (auto &Initializer : ConstructorDecl->inits()) {
-  if (Initializer->getSourceOrder() == -1) {
+for (const auto *Initializer : ConstructorDecl->inits()) {
+  if (!Initializer->isWritten()) {
 // Ignore implicit initializers.
 continue;
   }


Index: clang-rename/USRFinder.cpp
===
--- clang-rename/USRFinder.cpp
+++ clang-rename/USRFinder.cpp
@@ -91,8 +91,8 @@
   }
 
   bool VisitCXXConstructorDecl(clang::CXXConstructorDecl *ConstructorDecl) {
-for (auto &Initializer : ConstructorDecl->inits()) {
-  if (Initializer->getSourceOrder() == -1) {
+for (const auto *Initializer : ConstructorDecl->inits()) {
+  if (!Initializer->isWritten()) {
 // Ignore implicit initializers.
 continue;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D23272: [analyzer][Rewrite] Fix boxes and shadows in HTML rewrites in Firefox.

[Artem Dergachev via cfe-commits]

NoQ added inline comments.


Comment at: lib/Rewrite/HTMLRewrite.cpp:304
@@ -303,2 +303,3 @@
+  "  border-radius:5px;  box-shadow:1px 1px 7px #000; "
   "  -webkit-border-radius:5px;  -webkit-box-shadow:1px 1px 7px #000; "
   "position: absolute; top: -1em; left:10em; z-index: 1 } \n"

a.sidorin wrote:
> Should we remove '-webkit'-prefixed options as well? AFAIR, non-prefixed 
> options should be supported by Webkit. Could you check with Chrome/Safari?
All modern browsers should support the unprefixed property and work well 
without the prefixed property, but i'm not sure if there ever existed older 
browsers that used to support only the prefixed property. So i think i'm 
playing safe.

Also, i read that they recommend putting prefixed properties //before// 
unprefixed, because prefixed properties should override unprefixed properties 
whenever supported; i'd fix that.


https://reviews.llvm.org/D23272



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

Re: [PATCH] D23279: clang-reorder-fields

[Kirill Bobyrev via cfe-commits]

omtcyfz added a subscriber: omtcyfz.
omtcyfz added a comment.

In https://reviews.llvm.org/D23279#509047, @Eugene.Zelenko wrote:

> May be this could be Clang-rename mode?


Definitely not.

I think this is in scope of `clang-tidy`.

In https://reviews.llvm.org/D23279#509076, @compnerd wrote:

> This isn't really a renaming tool per se.  If you squint really hard, yes, it 
> does rename fields.  But, if we really want to save space, perhaps we should 
> collapse all the tools into `clang-tidy` or create a new `clang-refactor` 
> tool and just make the other things a part of that tool in various modes 
> (rename, reorder-fields, extract, etc) via sub-commands (a la git).  However, 
> I think thats a broader design decision which could be made outside the 
> context of this change.  However, if the concern is purely for install-time, 
> we could add components to the CMake install to control which of the extra 
> tools are built (note that this change doesn't even install the new binary!).


God, no. Please don't try to add over9000 tools. IMO this perfectly fits into 
`clang-tidy` scope. And it's not really `refactoring`.


Repository:
  rL LLVM

https://reviews.llvm.org/D23279



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

Re: [PATCH] D23279: clang-reorder-fields

[Kirill Bobyrev via cfe-commits]

omtcyfz added a comment.

And actually it makes much more sense for C than for C++. In C++ you just do 
`s/struct/class/g`, insert `public:` and you're golden.

P.S. It actually breaks code. I haven't looked into that, but

  bar::Foo foo = { &x, 17, 1.29, 0 }; // CHECK: bar::Foo foo = { 1.29, 0, 17, 
&x };

only works while it is in the same TU.

+ @alexfh on `clang-tidy` discussion.


Repository:
  rL LLVM

https://reviews.llvm.org/D23279



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

Re: [PATCH] D23279: clang-reorder-fields

[Manuel Klimek via cfe-commits]

klimek added a comment.

In https://reviews.llvm.org/D23279#509568, @omtcyfz wrote:

> And actually it makes much more sense for C than for C++. In C++ you just do 
> `s/struct/class/g`, insert `public:` and you're golden.


That doesn't work if you want to minimize object size, though?


Repository:
  rL LLVM

https://reviews.llvm.org/D23279



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

Re: [PATCH] D23298: [clang-rename] nit: use isWritten

[Kirill Bobyrev via cfe-commits]

omtcyfz added a comment.

Please fix the same thing @ `USRLocFinder.cpp:47`.

Other than that, looks OK - a bit more readable. Thanks.


https://reviews.llvm.org/D23298



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

Re: [PATCH] D23298: [clang-rename] nit: use isWritten

[Alexander Shaposhnikov via cfe-commits]

alexshap updated this revision to Diff 67301.
alexshap added a comment.

Fix clang-rename/USRLocFinder.cpp:47 as well


https://reviews.llvm.org/D23298

Files:
  clang-rename/USRFinder.cpp
  clang-rename/USRLocFinder.cpp

Index: clang-rename/USRLocFinder.cpp
===
--- clang-rename/USRLocFinder.cpp
+++ clang-rename/USRLocFinder.cpp
@@ -43,8 +43,8 @@
   // Declaration visitors:
 
   bool VisitCXXConstructorDecl(clang::CXXConstructorDecl *ConstructorDecl) {
-for (auto &Initializer : ConstructorDecl->inits()) {
-  if (Initializer->getSourceOrder() == -1) {
+for (const auto *Initializer : ConstructorDecl->inits()) {
+  if (!Initializer->isWritten()) {
 // Ignore implicit initializers.
 continue;
   }
Index: clang-rename/USRFinder.cpp
===
--- clang-rename/USRFinder.cpp
+++ clang-rename/USRFinder.cpp
@@ -91,8 +91,8 @@
   }
 
   bool VisitCXXConstructorDecl(clang::CXXConstructorDecl *ConstructorDecl) {
-for (auto &Initializer : ConstructorDecl->inits()) {
-  if (Initializer->getSourceOrder() == -1) {
+for (const auto *Initializer : ConstructorDecl->inits()) {
+  if (!Initializer->isWritten()) {
 // Ignore implicit initializers.
 continue;
   }


Index: clang-rename/USRLocFinder.cpp
===
--- clang-rename/USRLocFinder.cpp
+++ clang-rename/USRLocFinder.cpp
@@ -43,8 +43,8 @@
   // Declaration visitors:
 
   bool VisitCXXConstructorDecl(clang::CXXConstructorDecl *ConstructorDecl) {
-for (auto &Initializer : ConstructorDecl->inits()) {
-  if (Initializer->getSourceOrder() == -1) {
+for (const auto *Initializer : ConstructorDecl->inits()) {
+  if (!Initializer->isWritten()) {
 // Ignore implicit initializers.
 continue;
   }
Index: clang-rename/USRFinder.cpp
===
--- clang-rename/USRFinder.cpp
+++ clang-rename/USRFinder.cpp
@@ -91,8 +91,8 @@
   }
 
   bool VisitCXXConstructorDecl(clang::CXXConstructorDecl *ConstructorDecl) {
-for (auto &Initializer : ConstructorDecl->inits()) {
-  if (Initializer->getSourceOrder() == -1) {
+for (const auto *Initializer : ConstructorDecl->inits()) {
+  if (!Initializer->isWritten()) {
 // Ignore implicit initializers.
 continue;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D23279: clang-reorder-fields

[Kirill Bobyrev via cfe-commits]

omtcyfz added a comment.

In https://reviews.llvm.org/D23279#509567, @omtcyfz wrote:

> In https://reviews.llvm.org/D23279#509047, @Eugene.Zelenko wrote:
>
> > May be this could be Clang-rename mode?
>
>
> Definitely not.
>
> I think this is in scope of `clang-tidy`.
>
> In https://reviews.llvm.org/D23279#509076, @compnerd wrote:
>
> > This isn't really a renaming tool per se.  If you squint really hard, yes, 
> > it does rename fields.  But, if we really want to save space, perhaps we 
> > should collapse all the tools into `clang-tidy` or create a new 
> > `clang-refactor` tool and just make the other things a part of that tool in 
> > various modes (rename, reorder-fields, extract, etc) via sub-commands (a la 
> > git).  However, I think thats a broader design decision which could be made 
> > outside the context of this change.  However, if the concern is purely for 
> > install-time, we could add components to the CMake install to control which 
> > of the extra tools are built (note that this change doesn't even install 
> > the new binary!).
>
>
> God, no. Please don't try to add over9000 tools. IMO this perfectly fits into 
> `clang-tidy` scope. And it's not really `refactoring`.


Apologies, I didn't mean to be offensive.

My point, though, is, that we don't want to many tools in `clang-tools-extra` 
for many reasons. This exact tool doesn't fit into `clang-rename` scope, but at 
the same time it's not totally in scope of `clang-tidy` either.

It probably **makes** sense to have **clang-refactor** (or something) master 
tool, to which both tools would belong, but it's not as easy as a patch:

- it needs a good understanding of what would be there
- it needs a proper design

Both things are complex and require a Community-wise discussion.


Repository:
  rL LLVM

https://reviews.llvm.org/D23279



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

Re: [PATCH] D23279: clang-reorder-fields

[Kirill Bobyrev via cfe-commits]

omtcyfz added a comment.

In https://reviews.llvm.org/D23279#509570, @klimek wrote:

> In https://reviews.llvm.org/D23279#509568, @omtcyfz wrote:
>
> > And actually it makes much more sense for C than for C++. In C++ you just 
> > do `s/struct/class/g`, insert `public:` and you're golden.
>
>
> That doesn't work if you want to minimize object size, though?


Ah, yes.

In https://reviews.llvm.org/D23279#509606, @omtcyfz wrote:

> Both things are complex and require a Community-wise discussion.


I'll bring it up shortly.


Repository:
  rL LLVM

https://reviews.llvm.org/D23279



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

Re: [PATCH] D23298: [clang-rename] nit: use isWritten

[Kirill Bobyrev via cfe-commits]

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

LGTM

Thanks!

Do you have a commit access or do you need me to land the patch?


https://reviews.llvm.org/D23298



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

Re: [PATCH] D23298: [clang-rename] nit: use isWritten

[Alexander Shaposhnikov via cfe-commits]

alexshap added a comment.

> Do you have a commit access or do you need me to land the patch?


No, I don't. I will be grateful to you if you land the patch. Thanks.


https://reviews.llvm.org/D23298



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

Re: [PATCH] D23298: [clang-rename] nit: use isWritten

[Kirill Bobyrev via cfe-commits]

omtcyfz added a comment.

Sure.


https://reviews.llvm.org/D23298



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

r278110 - [analyzer] Try to fix coverity CID 1360469.

[Vassil Vassilev via cfe-commits]

Author: vvassilev
Date: Tue Aug  9 05:00:23 2016
New Revision: 278110

URL: http://llvm.org/viewvc/llvm-project?rev=278110&view=rev
Log:
[analyzer] Try to fix coverity CID 1360469.

Patch by Raphael Isemann!

Modified:
cfe/trunk/lib/Analysis/CloneDetection.cpp

Modified: cfe/trunk/lib/Analysis/CloneDetection.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CloneDetection.cpp?rev=278110&r1=278109&r2=278110&view=diff
==
--- cfe/trunk/lib/Analysis/CloneDetection.cpp (original)
+++ cfe/trunk/lib/Analysis/CloneDetection.cpp Tue Aug  9 05:00:23 2016
@@ -67,7 +67,7 @@ StmtSequence::iterator StmtSequence::beg
 
 StmtSequence::iterator StmtSequence::end() const {
   if (!holdsSequence()) {
-return &S + 1;
+return reinterpret_cast(&S) + 1;
   }
   auto CS = cast(S);
   return CS->body_begin() + EndIndex;


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

Re: [PATCH] D23279: clang-reorder-fields

[Manuel Klimek via cfe-commits]

klimek added a comment.

In https://reviews.llvm.org/D23279#509606, @omtcyfz wrote:

> In https://reviews.llvm.org/D23279#509567, @omtcyfz wrote:
>
> > In https://reviews.llvm.org/D23279#509047, @Eugene.Zelenko wrote:
> >
> > > May be this could be Clang-rename mode?
> >
> >
> > Definitely not.
> >
> > I think this is in scope of `clang-tidy`.
> >
> > In https://reviews.llvm.org/D23279#509076, @compnerd wrote:
> >
> > > This isn't really a renaming tool per se.  If you squint really hard, 
> > > yes, it does rename fields.  But, if we really want to save space, 
> > > perhaps we should collapse all the tools into `clang-tidy` or create a 
> > > new `clang-refactor` tool and just make the other things a part of that 
> > > tool in various modes (rename, reorder-fields, extract, etc) via 
> > > sub-commands (a la git).  However, I think thats a broader design 
> > > decision which could be made outside the context of this change.  
> > > However, if the concern is purely for install-time, we could add 
> > > components to the CMake install to control which of the extra tools are 
> > > built (note that this change doesn't even install the new binary!).
> >
> >
> > God, no. Please don't try to add over9000 tools. IMO this perfectly fits 
> > into `clang-tidy` scope. And it's not really `refactoring`.
>
>
> Apologies, I didn't mean to be offensive.
>
> My point, though, is, that we don't want to many tools in `clang-tools-extra` 
> for many reasons. This exact tool doesn't fit into `clang-rename` scope, but 
> at the same time it's not totally in scope of `clang-tidy` either.
>
> It probably **makes** sense to have **clang-refactor** (or something) master 
> tool, to which both tools would belong, but it's not as easy as a patch:
>
> - it needs a good understanding of what would be there
> - it needs a proper design
>
>   Both things are complex and require a Community-wise discussion.


Additionally (or alternatively) it might make sense to have a clang-tidy check 
that re-orders fields to minimize object size.


Repository:
  rL LLVM

https://reviews.llvm.org/D23279



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

[clang-tools-extra] r278111 - Fix Wdocumentation unknown parameter warning

[Simon Pilgrim via cfe-commits]

Author: rksimon
Date: Tue Aug  9 05:02:11 2016
New Revision: 278111

URL: http://llvm.org/viewvc/llvm-project?rev=278111&view=rev
Log:
Fix Wdocumentation unknown parameter warning

Modified:
clang-tools-extra/trunk/include-fixer/IncludeFixer.h

Modified: clang-tools-extra/trunk/include-fixer/IncludeFixer.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/IncludeFixer.h?rev=278111&r1=278110&r2=278111&view=diff
==
--- clang-tools-extra/trunk/include-fixer/IncludeFixer.h (original)
+++ clang-tools-extra/trunk/include-fixer/IncludeFixer.h Tue Aug  9 05:02:11 
2016
@@ -30,7 +30,7 @@ namespace include_fixer {
 class IncludeFixerActionFactory : public clang::tooling::ToolAction {
 public:
   /// \param SymbolIndexMgr A source for matching symbols to header files.
-  /// \param Context A context for the symbol being queried.
+  /// \param Contexts The contexts for the symbols being queried.
   /// \param StyleName Fallback style for reformatting.
   /// \param MinimizeIncludePaths whether inserted include paths are optimized.
   IncludeFixerActionFactory(SymbolIndexManager &SymbolIndexMgr,


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

[clang-tools-extra] r278112 - [clang-rename] cleanup: use isWritten

[Kirill Bobyrev via cfe-commits]

Author: omtcyfz
Date: Tue Aug  9 05:03:33 2016
New Revision: 278112

URL: http://llvm.org/viewvc/llvm-project?rev=278112&view=rev
Log:
[clang-rename] cleanup: use isWritten

nit: use isWritten and const auto *Initializer in
NamedDeclFindingASTVisitor::VisitCXXConstructorDecl method.
Test plan: make -j8 check-clang-tools (passed)

Patch by Alexander Shaposhnikov!

Reviewers: omtcyfz

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

Modified:
clang-tools-extra/trunk/clang-rename/USRFinder.cpp
clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp

Modified: clang-tools-extra/trunk/clang-rename/USRFinder.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-rename/USRFinder.cpp?rev=278112&r1=278111&r2=278112&view=diff
==
--- clang-tools-extra/trunk/clang-rename/USRFinder.cpp (original)
+++ clang-tools-extra/trunk/clang-rename/USRFinder.cpp Tue Aug  9 05:03:33 2016
@@ -91,8 +91,8 @@ public:
   }
 
   bool VisitCXXConstructorDecl(clang::CXXConstructorDecl *ConstructorDecl) {
-for (auto &Initializer : ConstructorDecl->inits()) {
-  if (Initializer->getSourceOrder() == -1) {
+for (const auto *Initializer : ConstructorDecl->inits()) {
+  if (!Initializer->isWritten()) {
 // Ignore implicit initializers.
 continue;
   }

Modified: clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp?rev=278112&r1=278111&r2=278112&view=diff
==
--- clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp (original)
+++ clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp Tue Aug  9 05:03:33 
2016
@@ -43,8 +43,8 @@ public:
   // Declaration visitors:
 
   bool VisitCXXConstructorDecl(clang::CXXConstructorDecl *ConstructorDecl) {
-for (auto &Initializer : ConstructorDecl->inits()) {
-  if (Initializer->getSourceOrder() == -1) {
+for (const auto *Initializer : ConstructorDecl->inits()) {
+  if (!Initializer->isWritten()) {
 // Ignore implicit initializers.
 continue;
   }


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

Re: [PATCH] D23298: [clang-rename] nit: use isWritten

[Kirill Bobyrev via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL278112: [clang-rename] cleanup: use isWritten (authored by 
omtcyfz).

Changed prior to commit:
  https://reviews.llvm.org/D23298?vs=67301&id=67309#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D23298

Files:
  clang-tools-extra/trunk/clang-rename/USRFinder.cpp
  clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp

Index: clang-tools-extra/trunk/clang-rename/USRFinder.cpp
===
--- clang-tools-extra/trunk/clang-rename/USRFinder.cpp
+++ clang-tools-extra/trunk/clang-rename/USRFinder.cpp
@@ -91,8 +91,8 @@
   }
 
   bool VisitCXXConstructorDecl(clang::CXXConstructorDecl *ConstructorDecl) {
-for (auto &Initializer : ConstructorDecl->inits()) {
-  if (Initializer->getSourceOrder() == -1) {
+for (const auto *Initializer : ConstructorDecl->inits()) {
+  if (!Initializer->isWritten()) {
 // Ignore implicit initializers.
 continue;
   }
Index: clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp
===
--- clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp
+++ clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp
@@ -43,8 +43,8 @@
   // Declaration visitors:
 
   bool VisitCXXConstructorDecl(clang::CXXConstructorDecl *ConstructorDecl) {
-for (auto &Initializer : ConstructorDecl->inits()) {
-  if (Initializer->getSourceOrder() == -1) {
+for (const auto *Initializer : ConstructorDecl->inits()) {
+  if (!Initializer->isWritten()) {
 // Ignore implicit initializers.
 continue;
   }


Index: clang-tools-extra/trunk/clang-rename/USRFinder.cpp
===
--- clang-tools-extra/trunk/clang-rename/USRFinder.cpp
+++ clang-tools-extra/trunk/clang-rename/USRFinder.cpp
@@ -91,8 +91,8 @@
   }
 
   bool VisitCXXConstructorDecl(clang::CXXConstructorDecl *ConstructorDecl) {
-for (auto &Initializer : ConstructorDecl->inits()) {
-  if (Initializer->getSourceOrder() == -1) {
+for (const auto *Initializer : ConstructorDecl->inits()) {
+  if (!Initializer->isWritten()) {
 // Ignore implicit initializers.
 continue;
   }
Index: clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp
===
--- clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp
+++ clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp
@@ -43,8 +43,8 @@
   // Declaration visitors:
 
   bool VisitCXXConstructorDecl(clang::CXXConstructorDecl *ConstructorDecl) {
-for (auto &Initializer : ConstructorDecl->inits()) {
-  if (Initializer->getSourceOrder() == -1) {
+for (const auto *Initializer : ConstructorDecl->inits()) {
+  if (!Initializer->isWritten()) {
 // Ignore implicit initializers.
 continue;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D23298: [clang-rename] nit: use isWritten

[Kirill Bobyrev via cfe-commits]

omtcyfz added a comment.

FYI: I think the diff (accessible by `Download Raw Diff`) is corrupted for some 
reason; at least I don't see `Index: test/clang-rename/USRLocFinder.cpp` line 
in the end, which should be present, I guess.


Repository:
  rL LLVM

https://reviews.llvm.org/D23298



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

Re: [PATCH] D23300: [analyzer] Add "Assuming..." diagnostic pieces for unsupported condition expressions.

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

xazax.hun added a comment.

In case there is a more complex condition does it only highligh the part that 
influenced taking the branch?

E.g.:
if (a || b) { // HIghlight only a, if a was true and b was not evaluated
}


https://reviews.llvm.org/D23300



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

Re: [PATCH] D10807: clang-format: Add SpaceAfterTemplate

[Sylvestre Ledru via cfe-commits]

sylvestre.ledru added a comment.

To be clear, this is blocking Mozilla to use much more clang-format


https://reviews.llvm.org/D10807



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

Re: [PATCH] D10807: clang-format: Add SpaceAfterTemplate

[Michael Rybakov via cfe-commits]

opilarium added a comment.

It's the huge deal for my company too. It would be nice if you reconsider.


https://reviews.llvm.org/D10807



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

Re: [PATCH] D10807: clang-format: Add SpaceAfterTemplate

[Michael Rybakov via cfe-commits]

opilarium added a comment.

Can we just accept this merge request? It's backward compatible and helps 
Mozilla to adapt clang-format.


https://reviews.llvm.org/D10807



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

Re: [PATCH] D10807: clang-format: Add SpaceAfterTemplate

[Daniel Jasper via cfe-commits]

djasper added a comment.

I still believe it's wrong to care about this in particular, but I also don't 
want to hold clang-format hostage for my own beliefs. Obviously, enough people 
care and the costs are limited.



Comment at: include/clang/Format/Format.h:356
@@ +355,3 @@
+  /// \brief If \c true, a space will be inserted after the 'template' keyword.
+  bool SpaceAfterTemplate;
+

Call this SpaceAfterTemplateKeyword.


https://reviews.llvm.org/D10807



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

Re: [PATCH] D10807: clang-format: Add SpaceAfterTemplate

[Sylvestre Ledru via cfe-commits]

sylvestre.ledru added a comment.

Thanks. I am rebasing and updating the patch.
I don't know if poiru is still around.


https://reviews.llvm.org/D10807



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

Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index

[Daniel Marjamäki via cfe-commits]

danielmarjamaki updated this revision to Diff 67322.
danielmarjamaki added a comment.

take care of review comments


https://reviews.llvm.org/D21134

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/MisplacedArrayIndexCheck.cpp
  clang-tidy/readability/MisplacedArrayIndexCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-misplaced-array-index.rst
  test/clang-tidy/readability-misplaced-array-index.cpp

Index: test/clang-tidy/readability-misplaced-array-index.cpp
===
--- test/clang-tidy/readability-misplaced-array-index.cpp
+++ test/clang-tidy/readability-misplaced-array-index.cpp
@@ -0,0 +1,32 @@
+// RUN: %check_clang_tidy %s readability-misplaced-array-index %t
+
+#define ABC  "abc"
+
+struct XY { int *x; int *y; };
+
+void dostuff(int);
+
+void unusualSyntax(int *x, struct XY *xy)
+{
+  10[x] = 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: confusing array subscript expression, usually the index is inside the []
+  // CHECK-FIXES: x[10] = 0;
+
+  10[xy->x] = 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: confusing array subscript expression, usually the index is inside the []
+  // CHECK-FIXES: xy->x[10] = 0;
+
+  dostuff(1["abc"]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: confusing array subscript expression, usually the index is inside the []
+  // CHECK-FIXES:  dostuff("abc"[1]);
+
+  dostuff(1[ABC]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: confusing array subscript expression, usually the index is inside the []
+  // No fixit, we don't want to replace with "abc"[1]
+}
+
+void normalSyntax(int *x)
+{
+  x[10] = 0;
+  dostuff(0[0 + ABC]);
+}
Index: docs/clang-tidy/checks/readability-misplaced-array-index.rst
===
--- docs/clang-tidy/checks/readability-misplaced-array-index.rst
+++ docs/clang-tidy/checks/readability-misplaced-array-index.rst
@@ -0,0 +1,29 @@
+.. title:: clang-tidy - readability-misplaced-array-index
+
+readability-misplaced-array-index
+=
+
+This check warns for unusual array index syntax.
+
+The following code has unusual array index syntax:
+
+.. code:: c++
+
+  void f(int *x, int y)
+  {
+y[x] = 0;
+  }
+
+becomes
+
+.. code:: c++
+
+  void f(int *x, int y)
+  {
+x[y] = 0;
+  }
+
+The check warns about such unusual syntax for readability reasons:
+ * There are programmers that are not familiar with this unusual syntax.
+ * It is possible that variables are mixed up.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -126,6 +126,7 @@
readability-identifier-naming
readability-implicit-bool-cast
readability-inconsistent-declaration-parameter-name
+   readability-misplaced-array-index
readability-named-parameter
readability-redundant-control-flow
readability-redundant-smartptr-get
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -75,6 +75,10 @@
   This check warns about the performance overhead arising from concatenating
   strings using the ``operator+``, instead of ``operator+=``.
 
+- New `readability-misplaced-array-index
+  `_ check
+
+  Warns when there is array index before the [] instead of inside it.
 
 Improvements to include-fixer
 -
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -19,6 +19,7 @@
 #include "IdentifierNamingCheck.h"
 #include "ImplicitBoolCastCheck.h"
 #include "InconsistentDeclarationParameterNameCheck.h"
+#include "MisplacedArrayIndexCheck.h"
 #include "NamedParameterCheck.h"
 #include "RedundantControlFlowCheck.h"
 #include "RedundantSmartptrGetCheck.h"
@@ -53,6 +54,8 @@
 "readability-implicit-bool-cast");
 CheckFactories.registerCheck(
 "readability-inconsistent-declaration-parameter-name");
+CheckFactories.registerCheck(
+"readability-misplaced-array-index");
 CheckFactories.registerCheck(
 "readability-static-definition-in-anonymous-namespace");
 CheckFactories.registerCheck(
Index: clang-tidy/readability/MisplacedArrayIndexCheck.h
===
--- clang-tidy/readability/MisplacedArrayIndexCheck.h
+++ clang-tidy/readability/MisplacedArrayIndexCheck.h
@@ -0,0 +1,36 @@
+//===--- MisplacedArrayIndexCheck.h - clang-tidy-*- C++ -*-===//
+//
+// The LLVM Compiler Infra

Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index

[Daniel Marjamäki via cfe-commits]

danielmarjamaki marked 4 inline comments as done.
danielmarjamaki added a comment.

as far as I see the only unsolved review comment now is about macros. if it can 
be handled better.



Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:28-29
@@ +27,4 @@
+  anyOf(stringLiteral(), declRefExpr(hasType(pointerType())),
+memberExpr(hasType(pointerType()))
+  .bind("expr"),
+  this);

Yes thanks!


Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:53
@@ +52,3 @@
+  if (hasMacroID(ArraySubscriptE) ||
+  
!Result.SourceManager->isWrittenInSameFile(ArraySubscriptE->getLocStart(),
+ ArraySubscriptE->getLocEnd()))

you can do lots of weird things with macros. so I wanted to just diagnose and 
then have no fixits that would be wrong etc. I removed the comment since I 
think the code is pretty clear.


Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:55
@@ +54,3 @@
+ ArraySubscriptE->getLocEnd()))
+return;
+

> What exactly is the recursive hasMacroID function trying to prevent? Do you 
> have a test that breaks if you just check that the start location of the 
> expression is not a macro?

I am worried about macros anywhere. I did not want to apply fixes for this 
code:  0[ABC]  but maybe using the Lexer would make that safe.

> In most cases, it's enough to check that the whole range is on the same macro 
> expansion level using Lexer::makeFileCharRange in order to prevent most 
> problems with macros, while allowing fixes in safe cases, e.g. replacements 
> in parameters of EXPECT_* macros from gtest.

I was very unsuccessful with that. I must do something wrong...
```
  CharSourceRange LRange = Lexer::makeFileCharRange(
  CharSourceRange::getCharRange(
  ArraySubscriptE->getLHS()->getSourceRange()),
  Result.Context->getSourceManager(), Result.Context->getLangOpts());

  CharSourceRange RRange = Lexer::makeFileCharRange(
  CharSourceRange::getCharRange(
  ArraySubscriptE->getLHS()->getSourceRange()),
  Result.Context->getSourceManager(), Result.Context->getLangOpts());

  StringRef LText = Lexer::getSourceText(LRange, *Result.SourceManager,
Result.Context->getLangOpts());

  StringRef RText = Lexer::getSourceText(RRange, *Result.SourceManager,
Result.Context->getLangOpts());

  Diag << 
FixItHint::CreateReplacement(ArraySubscriptE->getLHS()->getSourceRange(),
   RText);
  Diag << 
FixItHint::CreateReplacement(ArraySubscriptE->getRHS()->getSourceRange(),
LText);
```
No fixits worked with that code, with or without macros.

Do you see what I did wrong?


https://reviews.llvm.org/D21134



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

Re: [PATCH] D21695: [clang] Version support for UBSan handlers

[Filipe Cabecinhas via cfe-commits]

filcab added a comment.

Ping!


https://reviews.llvm.org/D21695



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

Re: [PATCH] D22725: [clang-tidy] Add check 'modernize-use-algorithm'

[Aaron Ballman via cfe-commits]

On Mon, Aug 8, 2016 at 11:36 PM, Piotr Padlewski
 wrote:
>
>
> 2016-08-08 8:33 GMT-07:00 Aaron Ballman :
>>
>> aaron.ballman added inline comments.
>>
>> 
>> Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:59-61
>> @@ +58,5 @@
>> +  IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
>> +  Options.get("IncludeStyle", "llvm"))) {
>> +
>> +  for (const auto &KeyValue :
>> +   {std::make_pair("memcpy", "std::copy"), {"memset", "std::fill"}})
>> {
>> 
>> alexfh wrote:
>> > I'm not sure this works on MSVC2013. Could someone try this before
>> > submitting?
>> It does not work in MSVC 2013.
>>
> What is the best way to fix it and still have a nice code?

As Alex pointed, out, just set the map values directly since there are only two.

>
>>
>> 
>> Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:102
>> @@ +101,3 @@
>> +  assert(it != Replacements.end() &&
>> + "Replacements list does not match list of registered matcher
>> names");
>> +  const std::string ReplacedName = it->second;
>> 
>> alexfh wrote:
>> > Notes are treated completely differently - each note has to be attached
>> > to a warning.
>> >
>> > Clang-tidy can only deal with two severity levels of diagnostics:
>> > "warning" and "error", but it's better to let them be controlled by the 
>> > user
>> > via `-warnings-as-errors=` command-line option or the `WarningsAsErrors`
>> > configuration file option.
>> Drat. I am not keen on this being a warning (let alone an error) because
>> it really doesn't warn the user against anything bad (the type safety
>> argument is tenuous at best), and it's arguable whether this actually
>> modernizes code. Do you think there's sufficient utility to this check to
>> warrant it being default-enabled as part of the modernize suite? For
>> instance, we have 368 instances of memcpy() and 171 instances of std::copy()
>> in the LLVM code base, which is an example of a very modern C++ code base.
>> I'm not opposed to the check, just worried that it will drown out
>> diagnostics that have more impact to the user.
>>
> I consider memcpy and memset very bugprone. It is very easy to swap
> arguments by mistake or pass something with a different type etc. Also it is
> easier to understand fill than a memset, so it is easier for
> C++ programmers who see any of it first time to get it.
> If I would see someone using memcpy or memset in C++ code on the review,
> asking to change for the one from the algorithm would be my first comment.

I think this boils down to personal preference, which is why I'm
concerned about the check. Either mechanism is correct, so this is
purely a stylistic check in many regards.

> About warnings - well, if someone choose this check to be run, then he
> probably wants to get warnings instead of notes.

The problem is that people don't always choose this check to be run,
they choose to run clang-tidy and this check is enabled by default. Or
they choose to run modernize and this check is enabled by default.

> I understand your point,
> that maybe we should use something lighter for the "light" mistakes, but the
> user is the one that feels if something is bad or not, and he just choose
> the check that he thinks it is resonable to run.
> I would like to see some proposal how you exactly you would imagine good
> warnings, but I don't think we should discuss it in this review. We can
> change it after it will be in the trunk.

As Alex pointed out, this is immaterial. Warnings are the proper
mechanism for this.

> Also, could you respond in the phabricator? I am not sure how does it work,
> but sometimes responding in a mail works fine and there is a comment in
> phab, but many times it doesn't appear there.

I generally do, but often email is easier (especially depending on
which machine I respond from). If there are issues with phab not
picking up comments from emails, we should alert the Phab developers,
as I understand that to be a bug.

~Aaron

>
> Piotr
>
>>
>> Repository:
>>   rL LLVM
>>
>> https://reviews.llvm.org/D22725
>>
>>
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.


Comment at: test/clang-tidy/readability-misplaced-array-index.cpp:31
@@ +30,3 @@
+  x[10] = 0;
+  dostuff(0[0 + ABC]);
+}

Why is this considered to be "normal syntax" and not worth getting a diagnostic?


https://reviews.llvm.org/D21134



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

Re: [PATCH] D10807: clang-format: Add SpaceAfterTemplate

[Michael Rybakov via cfe-commits]

opilarium added a comment.

> Thanks. I am rebasing and updating the patch.


Waiting for you.


https://reviews.llvm.org/D10807



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

Re: [PATCH] D18073: Add memory allocating functions

[Aaron Ballman via cfe-commits]

On Mon, Aug 8, 2016 at 8:13 PM, Alexander Riccio  wrote:
> ariccio added a comment.
>
> In https://reviews.llvm.org/D18073#504216, @dcoughlin wrote:
>
>> No. The identifier info will be null for C++ operators.
>
>
> I assume you mean `operator new/new[]/delete/delete[]

Correct.

~Aaron

>
>> > Thus, when`! isWindowsMSVCEnvironment`, I leave the Windows-only memory 
>> > allocating functions initialized to `nullptr`, which will never equal a 
>> > non-null `IdentifierInfo*`, and never trigger on a non-Windows platform.
>
>>
>
>>
>
>> It is probably better to be explicit about this.
>
>
>
>
>
> https://reviews.llvm.org/D18073
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index

[Daniel Marjamäki via cfe-commits]

danielmarjamaki marked 2 inline comments as done.


Comment at: test/clang-tidy/readability-misplaced-array-index.cpp:31
@@ +30,3 @@
+  x[10] = 0;
+  dostuff(0[0 + ABC]);
+}

aaron.ballman wrote:
> Why is this considered to be "normal syntax" and not worth getting a 
> diagnostic?
hmm.. I agree that it would be good to have a diagnostic for this code also.

I currently only diagnose when subscript is stringLiteral, declRefExpr or 
memberExpr. These are the cases when the code can be easily fixed by just 
replacing the expressions.

I will look into writing a diagnostic for this also.



https://reviews.llvm.org/D21134



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

Re: [PATCH] D23284: Add -Rpass-with-hotness

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.


Comment at: include/clang/Basic/DiagnosticDriverKinds.td:186
@@ +185,3 @@
+def warn_drv_argument_requires : Warning<
+  "argument '%0' requires %1">,
+  InGroup;

Given that there's only one driver option that requires this diagnostic, I 
would replace the %0 and %1 directives with their hard-coded text for right now 
(unless you plan on needing this generality in the near future). If we need to 
generalize the diagnostic in the future, we can do so using a %select directive.


https://reviews.llvm.org/D23284



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

Re: [PATCH] D21678: Fix For pr28288 - Error message in shift of vector values

[Vladimir Yakovlev via cfe-commits]

vbyakovl added inline comments.


Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8681-8683
@@ -8680,5 +8676,3 @@
 }
-return CheckVectorOperands(LHS, RHS, Loc, IsCompAssign,
-   /*AllowBothBool*/true,
-   /*AllowBoolConversions*/false);
   }

aaron.ballman wrote:
> Are you saying that calling `CheckVectorOperands` was always incorrect? Or 
> that it's no longer required because it should be fully covered by 
> `checkVectorShift`? Because the two methods do considerably different 
> checking, and I would have expected there to be more behavioral differences 
> in the tests by removing the call to `CheckVectorOperands` that suggests 
> there are tests missing.
Yes, calling CheckVectorOperands is not correct here because it compares 
operand types to be the same. For shift it is not true.


Repository:
  rL LLVM

https://reviews.llvm.org/D21678



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

Re: [PATCH] D21678: Fix For pr28288 - Error message in shift of vector values

[Aaron Ballman via cfe-commits]

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

LGTM, but I am also not well-versed in vector operations, so you may want to 
wait a day or two for @uweigand or someone else to sign off as well.



Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8681-8683
@@ -8680,5 +8676,3 @@
 }
-return CheckVectorOperands(LHS, RHS, Loc, IsCompAssign,
-   /*AllowBothBool*/true,
-   /*AllowBoolConversions*/false);
   }

vbyakovl wrote:
> aaron.ballman wrote:
> > Are you saying that calling `CheckVectorOperands` was always incorrect? Or 
> > that it's no longer required because it should be fully covered by 
> > `checkVectorShift`? Because the two methods do considerably different 
> > checking, and I would have expected there to be more behavioral differences 
> > in the tests by removing the call to `CheckVectorOperands` that suggests 
> > there are tests missing.
> Yes, calling CheckVectorOperands is not correct here because it compares 
> operand types to be the same. For shift it is not true.
Thank you for clarifying!


Repository:
  rL LLVM

https://reviews.llvm.org/D21678



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

Re: [PATCH] D17990: [clang-tidy] minor improvements in modernise-deprecated-headers check

[Kirill Bobyrev via cfe-commits]

omtcyf0 updated this revision to Diff 67328.
omtcyf0 added a comment.

Address comments Alex made.


https://reviews.llvm.org/D17990

Files:
  clang-tidy/modernize/DeprecatedHeadersCheck.cpp
  docs/clang-tidy/checks/modernize-deprecated-headers.rst
  test/clang-tidy/modernize-deprecated-headers-cxx03.cpp
  test/clang-tidy/modernize-deprecated-headers-cxx11.cpp

Index: test/clang-tidy/modernize-deprecated-headers-cxx11.cpp
===
--- test/clang-tidy/modernize-deprecated-headers-cxx11.cpp
+++ test/clang-tidy/modernize-deprecated-headers-cxx11.cpp
@@ -7,15 +7,12 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
 #include 
 #include 
-#include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -27,49 +24,43 @@
 #include 
 #include 
 
-// CHECK-MESSAGES: :[[@LINE-27]]:10: warning: inclusion of deprecated C++ header 'assert.h'; consider using 'cassert' instead [modernize-deprecated-headers]
-// CHECK-MESSAGES: :[[@LINE-27]]:10: warning: inclusion of deprecated C++ header 'complex.h'; consider using 'ccomplex' instead
-// CHECK-MESSAGES: :[[@LINE-27]]:10: warning: inclusion of deprecated C++ header 'ctype.h'; consider using 'cctype' instead
-// CHECK-MESSAGES: :[[@LINE-27]]:10: warning: inclusion of deprecated C++ header 'errno.h'; consider using 'cerrno' instead
-// CHECK-MESSAGES: :[[@LINE-27]]:10: warning: inclusion of deprecated C++ header 'fenv.h'; consider using 'cfenv' instead
-// CHECK-MESSAGES: :[[@LINE-27]]:10: warning: inclusion of deprecated C++ header 'float.h'; consider using 'cfloat' instead
-// CHECK-MESSAGES: :[[@LINE-27]]:10: warning: inclusion of deprecated C++ header 'inttypes.h'; consider using 'cinttypes' instead
-// CHECK-MESSAGES: :[[@LINE-27]]:10: warning: inclusion of deprecated C++ header 'iso646.h'; consider using 'ciso646' instead
-// CHECK-MESSAGES: :[[@LINE-27]]:10: warning: inclusion of deprecated C++ header 'limits.h'; consider using 'climits' instead
-// CHECK-MESSAGES: :[[@LINE-27]]:10: warning: inclusion of deprecated C++ header 'locale.h'; consider using 'clocale' instead
-// CHECK-MESSAGES: :[[@LINE-27]]:10: warning: inclusion of deprecated C++ header 'math.h'; consider using 'cmath' instead
-// CHECK-MESSAGES: :[[@LINE-27]]:10: warning: inclusion of deprecated C++ header 'setjmp.h'; consider using 'csetjmp' instead
-// CHECK-MESSAGES: :[[@LINE-27]]:10: warning: inclusion of deprecated C++ header 'signal.h'; consider using 'csignal' instead
-// CHECK-MESSAGES: :[[@LINE-27]]:10: warning: inclusion of deprecated C++ header 'stdalign.h'; consider using 'cstdalign' instead
-// CHECK-MESSAGES: :[[@LINE-27]]:10: warning: inclusion of deprecated C++ header 'stdarg.h'; consider using 'cstdarg' instead
-// CHECK-MESSAGES: :[[@LINE-27]]:10: warning: inclusion of deprecated C++ header 'stdbool.h'; consider using 'cstdbool' instead
-// CHECK-MESSAGES: :[[@LINE-27]]:10: warning: inclusion of deprecated C++ header 'stddef.h'; consider using 'cstddef' instead
-// CHECK-MESSAGES: :[[@LINE-27]]:10: warning: inclusion of deprecated C++ header 'stdint.h'; consider using 'cstdint' instead
-// CHECK-MESSAGES: :[[@LINE-27]]:10: warning: inclusion of deprecated C++ header 'stdio.h'; consider using 'cstdio' instead
-// CHECK-MESSAGES: :[[@LINE-27]]:10: warning: inclusion of deprecated C++ header 'stdlib.h'; consider using 'cstdlib' instead
-// CHECK-MESSAGES: :[[@LINE-27]]:10: warning: inclusion of deprecated C++ header 'string.h'; consider using 'cstring' instead
-// CHECK-MESSAGES: :[[@LINE-27]]:10: warning: inclusion of deprecated C++ header 'tgmath.h'; consider using 'ctgmath' instead
-// CHECK-MESSAGES: :[[@LINE-27]]:10: warning: inclusion of deprecated C++ header 'time.h'; consider using 'ctime' instead
-// CHECK-MESSAGES: :[[@LINE-27]]:10: warning: inclusion of deprecated C++ header 'uchar.h'; consider using 'cuchar' instead
-// CHECK-MESSAGES: :[[@LINE-27]]:10: warning: inclusion of deprecated C++ header 'wchar.h'; consider using 'cwchar' instead
-// CHECK-MESSAGES: :[[@LINE-27]]:10: warning: inclusion of deprecated C++ header 'wctype.h'; consider using 'cwctype' instead
+// CHECK-MESSAGES: :[[@LINE-24]]:10: warning: inclusion of deprecated C++ header 'assert.h'; consider using 'cassert' instead [modernize-deprecated-headers]
+// CHECK-MESSAGES: :[[@LINE-24]]:10: warning: inclusion of deprecated C++ header 'complex.h'; consider using 'complex' instead
+// CHECK-MESSAGES: :[[@LINE-24]]:10: warning: inclusion of deprecated C++ header 'ctype.h'; consider using 'cctype' instead
+// CHECK-MESSAGES: :[[@LINE-24]]:10: warning: inclusion of deprecated C++ header 'errno.h'; consider using 'cerrno' instead
+// CHECK-MESSAGES: :[[@LINE-24]]:10: warning: inclusion of deprecated C++ header 'fenv.h'; consider using 'cfenv' instead
+// CHECK-MESSAGES: :[[@LINE-24]]:10: warning: inclusion of deprecated C++ header 'float.h'; consider using 'cfloat' instead
+// CHECK-MESSAGES: :[[@LINE-24]]:10: warning: inclusio

Re: r266719 - Warn if function or variable cannot be implicitly instantiated

[Nico Weber via cfe-commits]

With 3.9 coming around, I'd like to suggest that we pull this warning from
3.9 and maybe trunk. It sounds like Sean found it only possible to deploy
this warning since they already had a workaround for a different compiler
bug (!). In Chromium, we can't enable this warning since one of our
(admittedly dubiously designed) template classes in a foundational library
requires people to have an explicit instantiation of their downstream
classes in a client cc file. Fixing this warning would mean giving the h
file in the foundational library forward declarations of all clients of the
template.

The motivation for this warning was PR24425, which is something that's only
relevant with modules enabled. Maybe the warning should only fire with
modules. But as-is, the warning warns about something that isn't really a
problem in practice, and it's difficult to fix (and as said, fixing it has
few benefits). I don't think it's at the bar we have for clang warnings.

Is there anyone who has deployed this warning successfully on a larger
codebase? Examples of real bugs it found?

On Fri, May 20, 2016 at 12:14 AM, Sean Silva  wrote:

>
>
> On Thu, May 19, 2016 at 12:13 PM, Serge Pavlov 
> wrote:
>
>> In this case moving implementation of `Singleton::getInstance()` into
>> a .cpp file would prevent compiler from instantiation of the method body
>> when it sees `Singleton::getInstance()`. In this case
>> `Singleton::getInstance()` must be instantiated in some source file
>> either explicitly or implicitly. If inline implementation for
>> `Singleton::getInstance()` is provided in the header, where
>> `Singleton` is defined, the method body is instantiated implicitly when
>> it is required. But the static field `instance` referenced in the method
>> still must be instantiated in some source file explicitly or implicitly. So
>> from viewpoint of convenience, moving the implementation of template
>> `Singleton::getInstance()` into source file does not look as more
>> inflexible solution.
>>
>> Probably it is more convenient to put the template for the static member
>> to header file too:
>>
>> template 
>> T *Singleton::instance = nullptr;
>>
>>
>> In this case both the method and corresponding static member are
>> instantiated implicitly by compiler, no warning occurs.
>> Can it be an acceptable solution?
>>
>
> I think that is what makes the most sense in this scenario (and it
> simplifies things for clients of the library).
> Unfortunately, for the library that was producing this warning they
> already required clients to provide explicit instantiations in a .cpp file
> (they had a macro like `DEFINE_SINGLETON_INSTANCE(T)` which a user would
> place in a .cpp file to instantiate Singleton::instance for their type).
>
> So fixing this warning like this would have a compatibility concern.
>
> Thankfully, it seems like some compilers (not clang) have a bug in that
> they will emit Singleton::instance any time that Singleton is
> instantiated unless they have already seen an explicit instantiation
> declaration of Singleton::instance in a header, so this library
> already had (under an #if) explicit instantiations for
> Singleton::instance for all classes that needed it in order to work
> around this compiler bug. So in the end I fixed this by just enabling those
> ifdefs so that clang would see the Singleton::instance explicitly
> declared in the header.
> (This is sort of a strange coincidence, but it worked out nicely)
>
>
>
>>
>> If there is intention to limit `Singleton` to some selected types,
>> explicit instantiation declaration may help. Adding the declarations like:
>>
>> extern template Foo *Singleton::instance;
>>
>>
>> prevents from warnings.
>>
>> Or you think that the message should propose moving static member
>> definition into header file as well?
>>
>
> I'm not sure, but I think that the current diagnostics are not very
> actionable. Hopefully google will lead users to your description in this
> thread. I wish we had something like https://doc.rust-lang.
> org/error-index.html where we could officially provide a more in-depth
> explanation similar to your posts in this thread.
>
> -- Sean Silva
>
>
>>
>>
>> Thanks,
>> --Serge
>>
>> 2016-05-19 9:15 GMT+06:00 Sean Silva :
>>
>>>
>>>
>>> On Thu, Apr 21, 2016 at 12:44 AM, Serge Pavlov 
>>> wrote:
>>>
 Let me demonstrate the problem using excerpt from v8 sources:

 -- lithium.h 
 template 
 struct LSubKindOperand {
   static int* Create(int index) { return &cache[index]; }
   static void SetUpCache();
   static int* cache;
 };

 struct LOperand {
   static void SetUpCaches();
 };

 #define LITHIUM_OPERAND_LIST(V)   \
   V(DoubleRegister,  1,   16)

 #define LITHIUM_TYPEDEF_SUBKIND_OPERAND_CLASS(name, type, number)   \
 typedef LSubKindOperand L##name;
 LITHIUM_OPERAND_LIST(LITHIUM_TYPEDEF_SUBKIND_OPERAND_CLASS)
 /* Ex

[PATCH] D23314: [analyzer] CloneDetector allows comparing clones for suspicious variable pattern errors.

[Raphael Isemann via cfe-commits]

teemperor created this revision.
teemperor added reviewers: v.g.vassilev, NoQ, zaks.anna.
teemperor added subscribers: cfe-commits, vsk.

One of the goals of the project was to find bugs caused by copy-pasting, which 
happen when a piece of code is copied but not all variables in this piece of 
code are correctly adapted to the new environment (e.g. like this 'if 
(mutex1.try_lock()) { foo(); mutex2.unlock(); }' where the mutex2 variable is a 
leftover from the original source code).

This patch adds support vor analyzing the pattern of the referenced variables 
for similar errors. So far only 'one-off' errors are supported, i.e. pattern 
errors where only one variable is not following the pattern of its related 
clones.

Additionally, the CloneChecker - which is currently the primary user of the 
CloneDetector API - now also supports reporting these suspicious clones.

https://reviews.llvm.org/D23314

Files:
  include/clang/Analysis/CloneDetection.h
  lib/Analysis/CloneDetection.cpp
  lib/StaticAnalyzer/Checkers/CloneChecker.cpp
  test/Analysis/copypaste/suspicious-clones.cpp

Index: test/Analysis/copypaste/suspicious-clones.cpp
===
--- /dev/null
+++ test/Analysis/copypaste/suspicious-clones.cpp
@@ -0,0 +1,97 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.clone.CloneChecker -analyzer-config alpha.clone.CloneChecker:ReportSuspiciousClones=true  -analyzer-config alpha.clone.CloneChecker:ReportNormalClones=false -verify %s
+
+// Tests finding a suspicious clone that references local variables.
+
+void log();
+
+int max(int a, int b) {
+  log();
+  if (a > b)
+return a;
+  return b; // expected-note{{Suggestion is based on the useage of this variable in a similar piece of code.}}
+}
+
+int maxClone(int x, int y, int z) {
+  log();
+  if (x > y)
+return x;
+  return z; // expected-warning{{Maybe you wanted to use 'y' here?}}
+}
+
+// Tests finding a suspicious clone that references global variables.
+
+struct mutex {
+  bool try_lock();
+  void unlock();
+};
+
+mutex m1;
+mutex m2;
+int i;
+
+void busyIncrement() {
+  while (true) {
+if (m1.try_lock()) {
+  ++i;
+  m1.unlock(); // expected-note{{Suggestion is based on the useage of this variable in a similar piece of code.}}
+  if (i > 1000) {
+return;
+  }
+}
+  }
+}
+
+void faultyBusyIncrement() {
+  while (true) {
+if (m1.try_lock()) {
+  ++i;
+  m2.unlock();  // expected-warning{{Maybe you wanted to use 'm1' here?}}
+  if (i > 1000) {
+return;
+  }
+}
+  }
+}
+
+// Tests that we provide two suggestions in cases where two fixes are possible.
+
+int foo(int a, int b, int c) {
+  a += b + c;
+  b /= a + b;
+  c -= b * a; // expected-warning{{Maybe you wanted to use 'a' here?}}
+  return c;
+}
+
+int fooClone(int a, int b, int c) {
+  a += b + c;
+  b /= a + b;
+  c -= a * a; // expected-note{{Or maybe you wanted to use 'b' here in this similar piece of code?}}
+  return c;
+}
+
+
+// Tests that for clone groups with a many possible suspicious clone pairs, at
+// most one warning per clone group is generated and every relevant clone is
+// reported through either a warning or a note.
+
+long bar1(long a, long b, long c, long d) {
+  c = a - b;
+  c = c / d * a;
+  d = b * b - c; // expected-warning{{Maybe you wanted to use 'c' here?}}
+  return d;
+}
+
+long bar2(long a, long b, long c, long d) {
+  c = a - b;
+  c = c / d * a;
+  d = c * b - c; // expected-note{{Or maybe you wanted to use 'b' here in this similar piece of code?}} \
+ // expected-warning{{Maybe you wanted to use 'a' here?}}
+  return d;
+}
+
+long bar3(long a, long b, long c, long d) {
+  c = a - b;
+  c = c / d * a;
+  d = a * b - c; // expected-note{{Or maybe you wanted to use 'c' here in this similar piece of code?}}
+  return d;
+}
Index: lib/StaticAnalyzer/Checkers/CloneChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/CloneChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CloneChecker.cpp
@@ -34,6 +34,83 @@
 
   void checkEndOfTranslationUnit(const TranslationUnitDecl *TU,
  AnalysisManager &Mgr, BugReporter &BR) const;
+
+  /// \brief Reports all clones to the user.
+  void reportClones(SourceManager &SM, AnalysisManager &Mgr,
+int MinComplexity) const {
+
+std::vector CloneGroups;
+CloneDetector.findClones(CloneGroups, MinComplexity);
+
+DiagnosticsEngine &DiagEngine = Mgr.getDiagnostic();
+
+unsigned WarnID = DiagEngine.getCustomDiagID(DiagnosticsEngine::Warning,
+ "Detected code clone.");
+
+unsigned NoteID = DiagEngine.getCustomDiagID(DiagnosticsEngine::Note,
+ "Related code clone is here.");
+
+for (CloneDetector::CloneGroup &Group : CloneGroups) {
+  // For readability reasons we sort the clones

Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index

[Jonathan Roelofs via cfe-commits]

jroelofs added a comment.

In https://reviews.llvm.org/D21134#509522, @danielmarjamaki wrote:

> In https://reviews.llvm.org/D21134#508524, @jroelofs wrote:
>
> > I think the replacement is wrong for something like:
> >
> >   int *arr; int offs1, offs2;
> >   offs1[arr + offs2] = 42;
> >
> >
> > which I think would give:
> >
> >   int *arr; int offs1, offs2;
> >   arr + offs2[offs1] = 42;
> >
> >
> > If the precedence of the "indexing" argument is less than that of the 
> > subscript operator, then you need to insert parens:
> >
> >   int *arr; int offs1, offs2;
> >   (arr + offs2)[offs1] = 42;
> >
>
>
> I don't think so. The matcher says:
>
>   hasRHS(ignoringParenImpCasts(
> anyOf(stringLiteral(), declRefExpr(hasType(pointsTo(qualType(,
>   memberExpr(hasType(pointsTo(qualType()))
>   
>
> I think it's hard to know what the replaced code should be. In some cases it 
> might be better with:
>
>   arr[offs1 + offs2] = 42;
>


Oh, yeah, you're right.


https://reviews.llvm.org/D21134



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

[PATCH] D23316: [analyzer] Fixed the false-positives caused by macro generated code.

[Raphael Isemann via cfe-commits]

teemperor created this revision.
teemperor added reviewers: v.g.vassilev, NoQ, zaks.anna.
teemperor added a subscriber: cfe-commits.

So far macro-generated code was treated by the CloneDetector as normal code. 
This caused that some macros where reported as false-positive clones because 
their generated code was detected as a clone.

This patch prevents that macro-generated code is influencing the complexity 
value of clones. This prevents that macros are reported as clones.

https://reviews.llvm.org/D23316

Files:
  lib/Analysis/CloneDetection.cpp
  test/Analysis/copypaste/macro-complexity.cpp
  test/Analysis/copypaste/macros.cpp

Index: test/Analysis/copypaste/macros.cpp
===
--- /dev/null
+++ test/Analysis/copypaste/macros.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -analyze -std=c++11 -analyzer-checker=alpha.clone.CloneChecker -verify %s
+
+// Tests that macros and non-macro clones aren't mixed into the same hash
+// group. This is currently necessary as all clones in a hash group need
+// to have the same complexity value. Macros have smaller complexity values
+// and need to be in their own hash group.
+
+#define ABS(a, b) a > b ? a : b;
+
+int max(int a, int b) { // expected-warning{{Detected code clone.}}
+  return a > b ? a : b;
+}
+
+int maxClone(int a, int b) { // expected-note{{Related code clone is here.}}
+  return a > b ? a : b;
+}
+
+int maxNoClone(int a, int b) {
+  return ABS(a, b);
+}
+
+// Tests that code containing macros is still reported.
+
+void longFoo() { // expected-warning{{Detected code clone.}}
+  const unsigned size = 10;
+  int array[size];
+  for (unsigned i = 0; i < size; i++) {
+array[i] = ABS(i, 5);
+  }
+}
+
+void longFooClone () { // expected-note{{Related code clone is here.}}
+  const unsigned size = 10;
+  int array[size];
+  for (unsigned i = 0; i < size; i++) {
+array[i] = ABS(i, 5);
+  }
+}
Index: test/Analysis/copypaste/macro-complexity.cpp
===
--- /dev/null
+++ test/Analysis/copypaste/macro-complexity.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -analyze -std=c++11 -analyzer-checker=alpha.clone.CloneChecker -analyzer-config alpha.clone.CloneChecker:MinimumCloneComplexity=20 -verify %s
+
+// Tests that only non-generated code increases the complexity of a statement.
+
+#define MACRO_FOO(a, b) a > b ? -a * a : -b * b;
+
+// The body of these two aren't complex enough to be considered clones of each
+// other.
+int fooMacro(int a, int b) {
+  return MACRO_FOO(a, b);
+}
+
+int fooMacroClone(int a, int b) {
+  return MACRO_FOO(a, b);
+}
+
+// This tests that above code would be complex enough to be reported.
+// Prevents that this tests won't accidentially pass because the code inside
+// the macro isn't complex enough.
+int foo(int a, int b) {  // expected-warning{{Detected code clone.}}
+  return a > b ? -a * a : -b * b;
+}
+
+int fooClone(int a, int b) {  // expected-note{{Related code clone is here.}}
+  return a > b ? -a * a : -b * b;
+}
Index: lib/Analysis/CloneDetection.cpp
===
--- lib/Analysis/CloneDetection.cpp
+++ lib/Analysis/CloneDetection.cpp
@@ -297,7 +297,14 @@
 ConstStmtVisitor::Visit##CLASS(S);  \
   }
 
-  DEF_ADD_DATA(Stmt, { addData(S->getStmtClass()); })
+  DEF_ADD_DATA(Stmt, {
+addData(S->getStmtClass());
+// This ensures that macro-code and non-macro code never lands in the same
+// hash group. This is necessary as they have different complexity values
+// and a single group currently needs to have one common complexity value.
+addData(S->getLocStart().isMacroID());
+addData(S->getLocEnd().isMacroID());
+  })
   DEF_ADD_DATA(Expr, { addData(S->getType()); })
 
   //--- Builtin functionality --//
@@ -419,6 +426,16 @@
 // Collect all relevant data from S and put it into the empty signature.
 StmtDataCollector(S, Context, Signature.Data);
 
+// If this code is generated by a macro, it won't increase the complexity
+// level of it's containing clone. This prevents false-positives caused by
+// complex macros. For example 'assert(false)' could be reported as a clone
+// if the assert macro is implemented as a expression that would
+// fulfill the standard complexity requirement for clones.
+bool IsInMacro = S->getLocStart().isMacroID() || S->getLocEnd().isMacroID();
+if (IsInMacro) {
+  Signature.Complexity = 0;
+}
+
 // Storage for the signatures of the direct child statements. This is only
 // needed if the current statement is a CompoundStmt.
 std::vector ChildSignatures;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17990: [clang-tidy] minor improvements in modernise-deprecated-headers check

[Kirill Bobyrev via cfe-commits]

omtcyf0 updated this revision to Diff 67334.
omtcyf0 marked 3 inline comments as done.
omtcyf0 added a comment.

Address remaining comments.


https://reviews.llvm.org/D17990

Files:
  clang-tidy/modernize/DeprecatedHeadersCheck.cpp
  docs/clang-tidy/checks/modernize-deprecated-headers.rst
  test/clang-tidy/modernize-deprecated-headers-cxx03.cpp
  test/clang-tidy/modernize-deprecated-headers-cxx11.cpp

Index: test/clang-tidy/modernize-deprecated-headers-cxx11.cpp
===
--- test/clang-tidy/modernize-deprecated-headers-cxx11.cpp
+++ test/clang-tidy/modernize-deprecated-headers-cxx11.cpp
@@ -1,163 +1,157 @@
 // RUN: %check_clang_tidy %s modernize-deprecated-headers %t -- -extra-arg-before=-isystem%S/Inputs/modernize-deprecated-headers -- -std=c++11 -v
 
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'assert.h'; consider using 'cassert' instead [modernize-deprecated-headers]
+// CHECK-FIXES: #include 
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'complex.h'; consider using 'complex' instead
+// CHECK-FIXES: #include 
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'ctype.h'; consider using 'cctype' instead
+// CHECK-FIXES: #include 
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'errno.h'; consider using 'cerrno' instead
+// CHECK-FIXES: #include 
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'fenv.h'; consider using 'cfenv' instead
+// CHECK-FIXES: #include 
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'float.h'; consider using 'cfloat' instead
+// CHECK-FIXES: #include 
 #include 
-#include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'inttypes.h'; consider using 'cinttypes' instead
+// CHECK-FIXES: #include 
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'limits.h'; consider using 'climits' instead
+// CHECK-FIXES: #include 
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'locale.h'; consider using 'clocale' instead
+// CHECK-FIXES: #include 
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'math.h'; consider using 'cmath' instead
+// CHECK-FIXES: #include 
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'setjmp.h'; consider using 'csetjmp' instead
+// CHECK-FIXES: #include 
 #include 
-#include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'signal.h'; consider using 'csignal' instead
+// CHECK-FIXES: #include 
 #include 
-#include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'stdarg.h'; consider using 'cstdarg' instead
+// CHECK-FIXES: #include 
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'stddef.h'; consider using 'cstddef' instead
+// CHECK-FIXES: #include 
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'stdint.h'; consider using 'cstdint' instead
+// CHECK-FIXES: #include 
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'stdio.h'; consider using 'cstdio' instead
+// CHECK-FIXES: #include 
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'stdlib.h'; consider using 'cstdlib' instead
+// CHECK-FIXES: #include 
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'string.h'; consider using 'cstring' instead
+// CHECK-FIXES: #include 
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'tgmath.h'; consider using 'ctgmath' instead
+// CHECK-FIXES: #include 
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'time.h'; consider using 'ctime' instead
+// CHECK-FIXES: #include 
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'uchar.h'; consider using 'cuchar' instead
+// CHECK-FIXES: #include 
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'wchar.h'; consider using 'cwchar' instead
+// CHECK-FIXES: #include 
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'wctype.h'; consider using 'cwctype' instead
+// CHECK-FIXES: #include 
 
-// CHECK-MESSAGES: :[[@LINE-27]]:10: warning: inclusion of deprecated C++ header 'assert.h'; consider using 'cassert' instead [modernize-deprecated-headers]
-// CHECK-MESSAGES: :[[@LINE-27]]:10: warning: inclusion of deprecated C++ header 'complex.h'; consider using 'ccomplex' instead
-// CHECK-MESSAGES: :[[@LINE-27]]:10: warning: inclusion of deprecated

Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index

[Daniel Marjamäki via cfe-commits]

danielmarjamaki updated this revision to Diff 67337.
danielmarjamaki added a comment.

More generic diagnostic. Diagnose all integerType[pointerType] expressions.


https://reviews.llvm.org/D21134

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/MisplacedArrayIndexCheck.cpp
  clang-tidy/readability/MisplacedArrayIndexCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-misplaced-array-index.rst
  test/clang-tidy/readability-misplaced-array-index.cpp

Index: test/clang-tidy/readability-misplaced-array-index.cpp
===
--- test/clang-tidy/readability-misplaced-array-index.cpp
+++ test/clang-tidy/readability-misplaced-array-index.cpp
@@ -0,0 +1,35 @@
+// RUN: %check_clang_tidy %s readability-misplaced-array-index %t
+
+#define ABC  "abc"
+
+struct XY { int *x; int *y; };
+
+void dostuff(int);
+
+void unusualSyntax(int *x, struct XY *xy)
+{
+  10[x] = 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: confusing array subscript expression, usually the index is inside the []
+  // CHECK-FIXES: x[10] = 0;
+
+  10[xy->x] = 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: confusing array subscript expression, usually the index is inside the []
+  // CHECK-FIXES: xy->x[10] = 0;
+
+  dostuff(1["abc"]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: confusing array subscript expression, usually the index is inside the []
+  // CHECK-FIXES:  dostuff("abc"[1]);
+
+  dostuff(1[ABC]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: confusing array subscript expression, usually the index is inside the []
+  // No fixit, we don't want to replace with "abc"[1]
+
+  dostuff(0[0 + ABC]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: confusing array subscript expression, usually the index is inside the []
+  // No fixit. Probably the code should be ABC[0]
+}
+
+void normalSyntax(int *x)
+{
+  x[10] = 0;
+}
Index: docs/clang-tidy/checks/readability-misplaced-array-index.rst
===
--- docs/clang-tidy/checks/readability-misplaced-array-index.rst
+++ docs/clang-tidy/checks/readability-misplaced-array-index.rst
@@ -0,0 +1,29 @@
+.. title:: clang-tidy - readability-misplaced-array-index
+
+readability-misplaced-array-index
+=
+
+This check warns for unusual array index syntax.
+
+The following code has unusual array index syntax:
+
+.. code:: c++
+
+  void f(int *x, int y)
+  {
+y[x] = 0;
+  }
+
+becomes
+
+.. code:: c++
+
+  void f(int *x, int y)
+  {
+x[y] = 0;
+  }
+
+The check warns about such unusual syntax for readability reasons:
+ * There are programmers that are not familiar with this unusual syntax.
+ * It is possible that variables are mixed up.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -126,6 +126,7 @@
readability-identifier-naming
readability-implicit-bool-cast
readability-inconsistent-declaration-parameter-name
+   readability-misplaced-array-index
readability-named-parameter
readability-redundant-control-flow
readability-redundant-smartptr-get
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -75,6 +75,10 @@
   This check warns about the performance overhead arising from concatenating
   strings using the ``operator+``, instead of ``operator+=``.
 
+- New `readability-misplaced-array-index
+  `_ check
+
+  Warns when there is array index before the [] instead of inside it.
 
 Improvements to include-fixer
 -
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -19,6 +19,7 @@
 #include "IdentifierNamingCheck.h"
 #include "ImplicitBoolCastCheck.h"
 #include "InconsistentDeclarationParameterNameCheck.h"
+#include "MisplacedArrayIndexCheck.h"
 #include "NamedParameterCheck.h"
 #include "RedundantControlFlowCheck.h"
 #include "RedundantSmartptrGetCheck.h"
@@ -53,6 +54,8 @@
 "readability-implicit-bool-cast");
 CheckFactories.registerCheck(
 "readability-inconsistent-declaration-parameter-name");
+CheckFactories.registerCheck(
+"readability-misplaced-array-index");
 CheckFactories.registerCheck(
 "readability-static-definition-in-anonymous-namespace");
 CheckFactories.registerCheck(
Index: clang-tidy/readability/MisplacedArrayIndexCheck.h
===
--- clang-tidy/readability/MisplacedA

Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index

[Daniel Marjamäki via cfe-commits]

danielmarjamaki updated this revision to Diff 67338.
danielmarjamaki added a comment.

ran clang-format


https://reviews.llvm.org/D21134

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/MisplacedArrayIndexCheck.cpp
  clang-tidy/readability/MisplacedArrayIndexCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-misplaced-array-index.rst
  test/clang-tidy/readability-misplaced-array-index.cpp

Index: test/clang-tidy/readability-misplaced-array-index.cpp
===
--- test/clang-tidy/readability-misplaced-array-index.cpp
+++ test/clang-tidy/readability-misplaced-array-index.cpp
@@ -0,0 +1,35 @@
+// RUN: %check_clang_tidy %s readability-misplaced-array-index %t
+
+#define ABC  "abc"
+
+struct XY { int *x; int *y; };
+
+void dostuff(int);
+
+void unusualSyntax(int *x, struct XY *xy)
+{
+  10[x] = 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: confusing array subscript expression, usually the index is inside the []
+  // CHECK-FIXES: x[10] = 0;
+
+  10[xy->x] = 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: confusing array subscript expression, usually the index is inside the []
+  // CHECK-FIXES: xy->x[10] = 0;
+
+  dostuff(1["abc"]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: confusing array subscript expression, usually the index is inside the []
+  // CHECK-FIXES:  dostuff("abc"[1]);
+
+  dostuff(1[ABC]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: confusing array subscript expression, usually the index is inside the []
+  // No fixit, we don't want to replace with "abc"[1]
+
+  dostuff(0[0 + ABC]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: confusing array subscript expression, usually the index is inside the []
+  // No fixit. Probably the code should be ABC[0]
+}
+
+void normalSyntax(int *x)
+{
+  x[10] = 0;
+}
Index: docs/clang-tidy/checks/readability-misplaced-array-index.rst
===
--- docs/clang-tidy/checks/readability-misplaced-array-index.rst
+++ docs/clang-tidy/checks/readability-misplaced-array-index.rst
@@ -0,0 +1,29 @@
+.. title:: clang-tidy - readability-misplaced-array-index
+
+readability-misplaced-array-index
+=
+
+This check warns for unusual array index syntax.
+
+The following code has unusual array index syntax:
+
+.. code:: c++
+
+  void f(int *x, int y)
+  {
+y[x] = 0;
+  }
+
+becomes
+
+.. code:: c++
+
+  void f(int *x, int y)
+  {
+x[y] = 0;
+  }
+
+The check warns about such unusual syntax for readability reasons:
+ * There are programmers that are not familiar with this unusual syntax.
+ * It is possible that variables are mixed up.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -126,6 +126,7 @@
readability-identifier-naming
readability-implicit-bool-cast
readability-inconsistent-declaration-parameter-name
+   readability-misplaced-array-index
readability-named-parameter
readability-redundant-control-flow
readability-redundant-smartptr-get
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -75,6 +75,10 @@
   This check warns about the performance overhead arising from concatenating
   strings using the ``operator+``, instead of ``operator+=``.
 
+- New `readability-misplaced-array-index
+  `_ check
+
+  Warns when there is array index before the [] instead of inside it.
 
 Improvements to include-fixer
 -
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -19,6 +19,7 @@
 #include "IdentifierNamingCheck.h"
 #include "ImplicitBoolCastCheck.h"
 #include "InconsistentDeclarationParameterNameCheck.h"
+#include "MisplacedArrayIndexCheck.h"
 #include "NamedParameterCheck.h"
 #include "RedundantControlFlowCheck.h"
 #include "RedundantSmartptrGetCheck.h"
@@ -53,6 +54,8 @@
 "readability-implicit-bool-cast");
 CheckFactories.registerCheck(
 "readability-inconsistent-declaration-parameter-name");
+CheckFactories.registerCheck(
+"readability-misplaced-array-index");
 CheckFactories.registerCheck(
 "readability-static-definition-in-anonymous-namespace");
 CheckFactories.registerCheck(
Index: clang-tidy/readability/MisplacedArrayIndexCheck.h
===
--- clang-tidy/readability/MisplacedArrayIndexCheck.h
+++ clang-tidy/readability/MisplacedArrayI

Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index

[Daniel Marjamäki via cfe-commits]

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

https://reviews.llvm.org/D21134



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

Re: [PATCH] D22946: [CUDA] Regression test to make sure C++ include path are forwarded to host and device frontends.

[Samuel Antao via cfe-commits]

sfantao updated this revision to Diff 67335.
sfantao marked an inline comment as done.
sfantao added a comment.

- Move CUDA C++ include path tests to cuda-detect.cu. Address other Art's 
comments.


https://reviews.llvm.org/D22946

Files:
  test/Driver/cuda-detect.cu

Index: test/Driver/cuda-detect.cu
===
--- test/Driver/cuda-detect.cu
+++ test/Driver/cuda-detect.cu
@@ -72,6 +72,14 @@
 // RUN:   | FileCheck %s -check-prefix COMMON \
 // RUN: -check-prefix NOCUDAINC -check-prefix NOLIBDEVICE
 
+// Verify that compiler accepts CUDA syntax with "-x cuda-cpp-output".
+// RUN: %clang -Werror -fsyntax-only -x cuda-cpp-output -c %s
+//
+// Verify that C++ include paths are passed for both host and device frontends.
+// RUN: %clang -### -target x86_64-linux-gnu %s \
+// RUN: --sysroot=%S/Inputs/ubuntu_14.04_multiarch_tree2 2>&1 \
+// RUN: | FileCheck %s --check-prefix CHECK-CXXINCLUDE
+
 // CHECK: Found CUDA installation: {{.*}}/Inputs/CUDA/usr/local/cuda
 // NOCUDA-NOT: Found CUDA installation:
 
@@ -92,3 +100,8 @@
 // CUDAINC-SAME: "-include" "__clang_cuda_runtime_wrapper.h"
 // NOCUDAINC-NOT: "-include" "__clang_cuda_runtime_wrapper.h"
 // COMMON-SAME: "-x" "cuda"
+// CHECK-CXXINCLUDE: clang{{.*}} "-cc1" "-triple" "nvptx64-nvidia-cuda"
+// CHECK-CXXINCLUDE-SAME: {{.*}}"-internal-isystem" "{{.+}}/include/c++/4.8"
+// CHECK-CXXINCLUDE: clang{{.*}} "-cc1" "-triple" "x86_64--linux-gnu"
+// CHECK-CXXINCLUDE-SAME: {{.*}}"-internal-isystem" "{{.+}}/include/c++/4.8"
+// CHECK-CXXINCLUDE: ld{{.*}}"


Index: test/Driver/cuda-detect.cu
===
--- test/Driver/cuda-detect.cu
+++ test/Driver/cuda-detect.cu
@@ -72,6 +72,14 @@
 // RUN:   | FileCheck %s -check-prefix COMMON \
 // RUN: -check-prefix NOCUDAINC -check-prefix NOLIBDEVICE
 
+// Verify that compiler accepts CUDA syntax with "-x cuda-cpp-output".
+// RUN: %clang -Werror -fsyntax-only -x cuda-cpp-output -c %s
+//
+// Verify that C++ include paths are passed for both host and device frontends.
+// RUN: %clang -### -target x86_64-linux-gnu %s \
+// RUN: --sysroot=%S/Inputs/ubuntu_14.04_multiarch_tree2 2>&1 \
+// RUN: | FileCheck %s --check-prefix CHECK-CXXINCLUDE
+
 // CHECK: Found CUDA installation: {{.*}}/Inputs/CUDA/usr/local/cuda
 // NOCUDA-NOT: Found CUDA installation:
 
@@ -92,3 +100,8 @@
 // CUDAINC-SAME: "-include" "__clang_cuda_runtime_wrapper.h"
 // NOCUDAINC-NOT: "-include" "__clang_cuda_runtime_wrapper.h"
 // COMMON-SAME: "-x" "cuda"
+// CHECK-CXXINCLUDE: clang{{.*}} "-cc1" "-triple" "nvptx64-nvidia-cuda"
+// CHECK-CXXINCLUDE-SAME: {{.*}}"-internal-isystem" "{{.+}}/include/c++/4.8"
+// CHECK-CXXINCLUDE: clang{{.*}} "-cc1" "-triple" "x86_64--linux-gnu"
+// CHECK-CXXINCLUDE-SAME: {{.*}}"-internal-isystem" "{{.+}}/include/c++/4.8"
+// CHECK-CXXINCLUDE: ld{{.*}}"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D23317: clang-format: Add SpaceAfterTemplate

[Sylvestre Ledru via cfe-commits]

sylvestre.ledru created this revision.
sylvestre.ledru added a reviewer: djasper.
sylvestre.ledru added subscribers: opilarium, cfe-commits, klimek.

This is required for compliance with the Mozilla style guide.

This is a rebase+minor change of Birunthan Mohanathas's patch


https://reviews.llvm.org/D23317

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -10191,6 +10191,7 @@
   CHECK_PARSE_BOOL(SpacesInContainerLiterals);
   CHECK_PARSE_BOOL(SpacesInCStyleCastParentheses);
   CHECK_PARSE_BOOL(SpaceAfterCStyleCast);
+  CHECK_PARSE_BOOL(SpaceAfterTemplateKeyword);
   CHECK_PARSE_BOOL(SpaceBeforeAssignmentOperators);
 
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterClass);
@@ -11329,6 +11330,12 @@
   verifyFormat("A>();", Spaces);
 }
 
+TEST_F(FormatTest, SpaceAfterTemplateKeyword) {
+  FormatStyle Style = getLLVMStyle();
+  Style.SpaceAfterTemplateKeyword = false;
+  verifyFormat("template void foo();", Style);
+}
+
 TEST_F(FormatTest, TripleAngleBrackets) {
   verifyFormat("f<<<1, 1>>>();");
   verifyFormat("f<<<1, 1, 1, s>>>();");
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -1984,9 +1984,10 @@
   if (Right.isOneOf(tok::semi, tok::comma))
 return false;
   if (Right.is(tok::less) &&
-  (Left.is(tok::kw_template) ||
-   (Line.Type == LT_ObjCDecl && Style.ObjCSpaceBeforeProtocolList)))
+  Line.Type == LT_ObjCDecl && Style.ObjCSpaceBeforeProtocolList)
 return true;
+  if (Right.is(tok::less) && Left.is(tok::kw_template))
+return Style.SpaceAfterTemplateKeyword;
   if (Left.isOneOf(tok::exclaim, tok::tilde))
 return false;
   if (Left.is(tok::at) &&
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -338,6 +338,7 @@
 IO.mapOptional("ReflowComments", Style.ReflowComments);
 IO.mapOptional("SortIncludes", Style.SortIncludes);
 IO.mapOptional("SpaceAfterCStyleCast", Style.SpaceAfterCStyleCast);
+IO.mapOptional("SpaceAfterTemplateKeyword", Style.SpaceAfterTemplateKeyword);
 IO.mapOptional("SpaceBeforeAssignmentOperators",
Style.SpaceBeforeAssignmentOperators);
 IO.mapOptional("SpaceBeforeParens", Style.SpaceBeforeParens);
@@ -552,6 +553,7 @@
   LLVMStyle.SpacesInContainerLiterals = true;
   LLVMStyle.SpacesInCStyleCastParentheses = false;
   LLVMStyle.SpaceAfterCStyleCast = false;
+  LLVMStyle.SpaceAfterTemplateKeyword = true;
   LLVMStyle.SpaceBeforeParens = FormatStyle::SBPO_ControlStatements;
   LLVMStyle.SpaceBeforeAssignmentOperators = true;
   LLVMStyle.SpacesInAngles = false;
@@ -663,6 +665,7 @@
   MozillaStyle.ObjCSpaceBeforeProtocolList = false;
   MozillaStyle.PenaltyReturnTypeOnItsOwnLine = 200;
   MozillaStyle.PointerAlignment = FormatStyle::PAS_Left;
+  MozillaStyle.SpaceAfterTemplateKeyword = false;
   return MozillaStyle;
 }
 
Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -549,6 +549,9 @@
   /// \brief If ``true``, a space may be inserted after C style casts.
   bool SpaceAfterCStyleCast;
 
+  /// \brief If \c true, a space will be inserted after the 'template' keyword.
+  bool SpaceAfterTemplateKeyword;
+
   /// \brief If ``false``, spaces will be removed before assignment operators.
   bool SpaceBeforeAssignmentOperators;
 
@@ -698,6 +701,7 @@
PenaltyReturnTypeOnItsOwnLine == R.PenaltyReturnTypeOnItsOwnLine &&
PointerAlignment == R.PointerAlignment &&
SpaceAfterCStyleCast == R.SpaceAfterCStyleCast &&
+   SpaceAfterTemplateKeyword == R.SpaceAfterTemplateKeyword &&
SpaceBeforeAssignmentOperators == R.SpaceBeforeAssignmentOperators &&
SpaceBeforeParens == R.SpaceBeforeParens &&
SpaceInEmptyParentheses == R.SpaceInEmptyParentheses &&
Index: docs/ClangFormatStyleOptions.rst
===
--- docs/ClangFormatStyleOptions.rst
+++ docs/ClangFormatStyleOptions.rst
@@ -670,6 +670,9 @@
 **SpaceAfterCStyleCast** (``bool``)
   If ``true``, a space may be inserted after C style casts.
 
+**SpaceAfterTemplateKeyword** (``bool``)
+  If ``true``, a space will be inserted after the 'template' keyword.
+
 **SpaceBeforeAssignmentOperators** (``bool``)
   If ``false``, spaces will be removed before assignment operators.
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/

Re: [PATCH] D10807: clang-format: Add SpaceAfterTemplate

[Sylvestre Ledru via cfe-commits]

sylvestre.ledru added a comment.

Done in https://reviews.llvm.org/D23317


https://reviews.llvm.org/D10807



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

Re: [PATCH] D23317: clang-format: Add SpaceAfterTemplate

[Daniel Jasper via cfe-commits]

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

Looks good.


https://reviews.llvm.org/D23317



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

Re: [PATCH] D23317: clang-format: Add SpaceAfterTemplate

[Sylvestre Ledru via cfe-commits]

sylvestre.ledru added a comment.

Thanks!
I don't think you want it but do you want me to add this to the 4.0 release 
notes?


https://reviews.llvm.org/D23317



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

r278121 - clang-format: Add SpaceAfterTemplate

[Sylvestre Ledru via cfe-commits]

Author: sylvestre
Date: Tue Aug  9 09:24:40 2016
New Revision: 278121

URL: http://llvm.org/viewvc/llvm-project?rev=278121&view=rev
Log:
clang-format: Add SpaceAfterTemplate

Summary:
This is required for compliance with the Mozilla style guide.

This is a rebase+minor change of Birunthan Mohanathas's patch


Reviewers: djasper

Subscribers: klimek, cfe-commits, opilarium

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

Modified:
cfe/trunk/docs/ClangFormatStyleOptions.rst
cfe/trunk/include/clang/Format/Format.h
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/docs/ClangFormatStyleOptions.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangFormatStyleOptions.rst?rev=278121&r1=278120&r2=278121&view=diff
==
--- cfe/trunk/docs/ClangFormatStyleOptions.rst (original)
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst Tue Aug  9 09:24:40 2016
@@ -670,6 +670,9 @@ the configuration (without a prefix: ``A
 **SpaceAfterCStyleCast** (``bool``)
   If ``true``, a space may be inserted after C style casts.
 
+**SpaceAfterTemplateKeyword** (``bool``)
+  If ``true``, a space will be inserted after the 'template' keyword.
+
 **SpaceBeforeAssignmentOperators** (``bool``)
   If ``false``, spaces will be removed before assignment operators.
 

Modified: cfe/trunk/include/clang/Format/Format.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=278121&r1=278120&r2=278121&view=diff
==
--- cfe/trunk/include/clang/Format/Format.h (original)
+++ cfe/trunk/include/clang/Format/Format.h Tue Aug  9 09:24:40 2016
@@ -549,6 +549,9 @@ struct FormatStyle {
   /// \brief If ``true``, a space may be inserted after C style casts.
   bool SpaceAfterCStyleCast;
 
+  /// \brief If \c true, a space will be inserted after the 'template' keyword.
+  bool SpaceAfterTemplateKeyword;
+
   /// \brief If ``false``, spaces will be removed before assignment operators.
   bool SpaceBeforeAssignmentOperators;
 
@@ -698,6 +701,7 @@ struct FormatStyle {
PenaltyReturnTypeOnItsOwnLine == R.PenaltyReturnTypeOnItsOwnLine &&
PointerAlignment == R.PointerAlignment &&
SpaceAfterCStyleCast == R.SpaceAfterCStyleCast &&
+   SpaceAfterTemplateKeyword == R.SpaceAfterTemplateKeyword &&
SpaceBeforeAssignmentOperators == R.SpaceBeforeAssignmentOperators 
&&
SpaceBeforeParens == R.SpaceBeforeParens &&
SpaceInEmptyParentheses == R.SpaceInEmptyParentheses &&

Modified: cfe/trunk/lib/Format/Format.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=278121&r1=278120&r2=278121&view=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Tue Aug  9 09:24:40 2016
@@ -338,6 +338,7 @@ template <> struct MappingTraitshttp://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=278121&r1=278120&r2=278121&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Tue Aug  9 09:24:40 2016
@@ -1984,9 +1984,10 @@ bool TokenAnnotator::spaceRequiredBetwee
   if (Right.isOneOf(tok::semi, tok::comma))
 return false;
   if (Right.is(tok::less) &&
-  (Left.is(tok::kw_template) ||
-   (Line.Type == LT_ObjCDecl && Style.ObjCSpaceBeforeProtocolList)))
+  Line.Type == LT_ObjCDecl && Style.ObjCSpaceBeforeProtocolList)
 return true;
+  if (Right.is(tok::less) && Left.is(tok::kw_template))
+return Style.SpaceAfterTemplateKeyword;
   if (Left.isOneOf(tok::exclaim, tok::tilde))
 return false;
   if (Left.is(tok::at) &&

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=278121&r1=278120&r2=278121&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Tue Aug  9 09:24:40 2016
@@ -10191,6 +10191,7 @@ TEST_F(FormatTest, ParsesConfigurationBo
   CHECK_PARSE_BOOL(SpacesInContainerLiterals);
   CHECK_PARSE_BOOL(SpacesInCStyleCastParentheses);
   CHECK_PARSE_BOOL(SpaceAfterCStyleCast);
+  CHECK_PARSE_BOOL(SpaceAfterTemplateKeyword);
   CHECK_PARSE_BOOL(SpaceBeforeAssignmentOperators);
 
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterClass);
@@ -11329,6 +11330,12 @@ TEST_F(FormatTest, SpacesInAngles) {
   verifyFormat("A>();", Spaces);
 }
 
+TEST_F(FormatTest, SpaceAfterTemplateKeyword) {
+  FormatStyle Style = getLLVMStyle();
+  Style.SpaceAfterTemplateKeyword = false;
+  verifyFormat("template 

Re: [PATCH] D23279: clang-reorder-fields

[Saleem Abdulrasool via cfe-commits]

compnerd added a comment.

If the argument really is that we want to minimize the tools then Id argue that 
`clang-rename` also belongs in `clang-tidy` as it would be used to rename 
fields to match the naming convention (tidying up your code base).

`clang-tidy` could work, but it does seem to be introducing a completely new 
concept into `clang-tidy` AFAICT.  It has so far only done equivalent changes.  
This operation doesn't guarantee equivalence: if you are doing a 
`reinterpret_cast` or a C-style cast, that will no longer work as the object 
layout has changed.

We could merge both tools into `clang-tidy`, or perhaps we can hold off on that 
for the wider discussion, and allow this to make progress in the mean time.


Repository:
  rL LLVM

https://reviews.llvm.org/D23279



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

Re: [PATCH] D23265: [clang-tidy] enhance readability-else-after-return

[Kirill Bobyrev via cfe-commits]

omtcyfz updated this revision to Diff 67347.
omtcyfz marked 2 inline comments as done.
omtcyfz added a comment.

Address the comments.


https://reviews.llvm.org/D23265

Files:
  clang-tidy/readability/ElseAfterReturnCheck.cpp
  docs/clang-tidy/checks/readability-else-after-return.rst
  test/clang-tidy/readability-else-after-return.cpp

Index: test/clang-tidy/readability-else-after-return.cpp
===
--- test/clang-tidy/readability-else-after-return.cpp
+++ test/clang-tidy/readability-else-after-return.cpp
@@ -4,15 +4,15 @@
   if (a > 0)
 return;
   else // comment
-// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: don't use else after return
-// CHECK-FIXES: {{^}}  // comment
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use 'else' after 'return'
+  // CHECK-FIXES: // comment
 return;
 
   if (a > 0) {
 return;
   } else { // comment
-// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: don't use else after return
-// CHECK-FIXES:  } // comment
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return'
+  // CHECK-FIXES: // comment
 return;
   }
 
@@ -28,7 +28,42 @@
 f(0);
   else if (a > 10)
 return;
-  else
+  else // comment
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use 'else' after 'return'
+  // CHECK-FIXES: // comment
 f(0);
 }
 
+void foo() {
+label:
+  for (unsigned x = 0; x < 42; ++x) {
+if (x) {
+  continue;
+} else { // comment
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'continue'
+// CHECK-FIXES: // comment
+  x++;
+}
+if (x) {
+  break;
+} else { // comment
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'break'
+// CHECK-FIXES: // comment
+  x++;
+}
+if (x) {
+  throw 42;
+} else { // comment
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'throw'
+// CHECK-FIXES: // comment
+  x++;
+}
+if (x) {
+  goto label;
+} else { // comment
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'goto'
+// CHECK-FIXES: // comment
+  x++;
+}
+  }
+}
Index: docs/clang-tidy/checks/readability-else-after-return.rst
===
--- docs/clang-tidy/checks/readability-else-after-return.rst
+++ docs/clang-tidy/checks/readability-else-after-return.rst
@@ -3,7 +3,72 @@
 readability-else-after-return
 =
 
+`LLVM Coding Standards `_ advises to
+reduce indentation where possible and where it makes understanding code easier.
+Early exit is one of the suggested enforcement of that. Please do not use `else`
+or `else if` after something that interrupts control flow - like `return`,
+`break`, `continue`, `throw`, `goto`, etc.
 
-Flags the usages of ``else`` after ``return``.
+Following piece of code illustrates how check would work. This piece of code:
 
-http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return
+``` c++
+void foo(int Value) {
+  int Local = 0;
+  for (int i = 0; i < 42; i++) {
+if (Value == 1) {
+  return;
+} else {
+  Local++;
+}
+
+if (Value == 2)
+  continue;
+else
+  Local++;
+
+if (Value == 3) {
+  throw 42;
+} else {
+  Local++;
+}
+
+if (Value == 4) {
+  goto label;
+} else {
+  Local++;
+}
+  }
+}
+```
+
+Would be transformed into:
+
+``` c++
+void foo(int Value) {
+  int Local = 0;
+  for (int i = 0; i < 42; i++) {
+if (Value == 1) {
+  return;
+}
+Local++;
+
+if (Value == 2)
+  continue;
+Local++;
+
+if (Value == 3) {
+  throw 42;
+}
+Local++;
+
+if (Value == 4) {
+  goto label;
+}
+Local++;
+  }
+}
+```
+
+
+This checks helps to enforce this `Coding Standars recommendation
+`_.
Index: clang-tidy/readability/ElseAfterReturnCheck.cpp
===
--- clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -19,20 +19,37 @@
 namespace readability {
 
 void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) {
-  // FIXME: Support continue, break and throw.
   Finder->addMatcher(
-  compoundStmt(
-  forEach(ifStmt(hasThen(stmt(anyOf(returnStmt(),
-compoundStmt(has(returnStmt()),
+  stmt(
+  forEach(ifStmt(hasThen(stmt(anyOf(
+ returnStmt().bind("return"),
+ compoundStmt(has(returnStmt().bind("return"))),
+ continueStmt().bind("continue"),
+ compoundStmt(has(continueStmt().bind("continue"))),
+ breakStmt().bind("break"),
+ 

[PATCH] D23320: [analyzer] Fixed crash in CloneDetector in function calls.

[Raphael Isemann via cfe-commits]

teemperor created this revision.
teemperor added reviewers: v.g.vassilev, NoQ, zaks.anna.
teemperor added subscribers: cfe-commits, vsk.

StmtDataCollector currently crashes on function calls because they don't have a 
callee. This patch fixes this.

https://reviews.llvm.org/D23320

Files:
  lib/Analysis/CloneDetection.cpp
  test/Analysis/copypaste/call.cpp

Index: test/Analysis/copypaste/call.cpp
===
--- test/Analysis/copypaste/call.cpp
+++ test/Analysis/copypaste/call.cpp
@@ -22,3 +22,15 @@
 return b();
   return true;
 }
+
+// Test that we don't crash on function pointer calls
+
+bool (*funcPtr)(int);
+
+bool fooPtr1(int x) {
+  if (x > 0)
+return false;
+  else if (x < 0)
+return funcPtr(1);
+  return true;
+}
Index: lib/Analysis/CloneDetection.cpp
===
--- lib/Analysis/CloneDetection.cpp
+++ lib/Analysis/CloneDetection.cpp
@@ -318,8 +318,11 @@
   })
 
   //--- Calls --//
-  DEF_ADD_DATA(CallExpr,
-   { addData(S->getDirectCallee()->getQualifiedNameAsString()); })
+  DEF_ADD_DATA(CallExpr, {
+// Function pointers don't have a callee and we just skip hashing it.
+if (S->getDirectCallee())
+  addData(S->getDirectCallee()->getQualifiedNameAsString());
+  })
 
   //--- Exceptions -//
   DEF_ADD_DATA(CXXCatchStmt, { addData(S->getCaughtType()); })


Index: test/Analysis/copypaste/call.cpp
===
--- test/Analysis/copypaste/call.cpp
+++ test/Analysis/copypaste/call.cpp
@@ -22,3 +22,15 @@
 return b();
   return true;
 }
+
+// Test that we don't crash on function pointer calls
+
+bool (*funcPtr)(int);
+
+bool fooPtr1(int x) {
+  if (x > 0)
+return false;
+  else if (x < 0)
+return funcPtr(1);
+  return true;
+}
Index: lib/Analysis/CloneDetection.cpp
===
--- lib/Analysis/CloneDetection.cpp
+++ lib/Analysis/CloneDetection.cpp
@@ -318,8 +318,11 @@
   })
 
   //--- Calls --//
-  DEF_ADD_DATA(CallExpr,
-   { addData(S->getDirectCallee()->getQualifiedNameAsString()); })
+  DEF_ADD_DATA(CallExpr, {
+// Function pointers don't have a callee and we just skip hashing it.
+if (S->getDirectCallee())
+  addData(S->getDirectCallee()->getQualifiedNameAsString());
+  })
 
   //--- Exceptions -//
   DEF_ADD_DATA(CXXCatchStmt, { addData(S->getCaughtType()); })
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [libcxx] r278032 - CMakeLists.txt cleanups: synchronize version with rest of LLVM, consistent spacing.

[Saleem Abdulrasool via cfe-commits]

On Mon, Aug 8, 2016 at 1:20 PM, Hans Wennborg  wrote:

> I didn't merge this one to 3.9 because the PACKAGE_VERSION seems to
> have always been trunk-svn, and it's at least not as misleading as
> having the wrong number.
>
> I wonder if PACKAGE_VERSION is actually important for anything in
> libcxx, libcxxabi and libunwind? At least for libcxx, the important
> version is _LIBCPP_VERSION in include/__config.
>

I don't believe we use `PACKAGE_VERSION`.  I can see it being useful though
as CPack would use that value.


> Thanks,
> Hans
>
> On Mon, Aug 8, 2016 at 11:01 AM, Eugene Zelenko via cfe-commits
>  wrote:
> > Author: eugenezelenko
> > Date: Mon Aug  8 13:01:50 2016
> > New Revision: 278032
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=278032&view=rev
> > Log:
> > CMakeLists.txt cleanups: synchronize version with rest of LLVM,
> consistent spacing.
> >
> > Differential revision: https://reviews.llvm.org/D23091
> >
> > Modified:
> > libcxx/trunk/CMakeLists.txt
> >
> > Modified: libcxx/trunk/CMakeLists.txt
> > URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/
> CMakeLists.txt?rev=278032&r1=278031&r2=278032&view=diff
> > 
> ==
> > --- libcxx/trunk/CMakeLists.txt (original)
> > +++ libcxx/trunk/CMakeLists.txt Mon Aug  8 13:01:50 2016
> > @@ -26,10 +26,10 @@ if (LIBCXX_BUILT_STANDALONE)
> >project(libcxx CXX C)
> >
> >set(PACKAGE_NAME libcxx)
> > -  set(PACKAGE_VERSION trunk-svn)
> > +  set(PACKAGE_VERSION 4.0.0svn)
> >set(PACKAGE_STRING "${PACKAGE_NAME} ${PACKAGE_VERSION}")
> >set(PACKAGE_BUGREPORT "llvm-b...@lists.llvm.org")
> > -endif ()
> > +endif()
>



-- 
Saleem Abdulrasool
compnerd (at) compnerd (dot) org
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22666: Frontend: Fix mcount inlining bug

[Saleem Abdulrasool via cfe-commits]

compnerd added a comment.

Theres a frontend mangler and a backend mangler.  You should try this on 
Windows GNU environments :-).


https://reviews.llvm.org/D22666



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

Re: [PATCH] D23198: clang-rename rename-all: support reading old/newname pairs from a YAML file

[Kirill Bobyrev via cfe-commits]

omtcyfz added inline comments.


Comment at: clang-rename/tool/ClangRename.cpp:65
@@ +64,3 @@
+
+  RenameAllInfo() : Offset(0) {}
+};

AFAIK there's no need to do that, integer types are by default initialized with 
0, aren't they?


https://reviews.llvm.org/D23198



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

Re: [PATCH] D23300: [analyzer] Add "Assuming..." diagnostic pieces for unsupported condition expressions.

[Artem Dergachev via cfe-commits]

NoQ added a comment.

In https://reviews.llvm.org/D23300#509665, @xazax.hun wrote:

> In case there is a more complex condition does it only highligh the part that 
> influenced taking the branch?
>
> E.g.:
>
>   if (a || b) { // HIghlight only a, if a was true and b was not evaluated
>   } 
>


Not yet, and this part of things is still broken - perhaps more patches would 
be needed to address all the issues: F2256900: report-79b22c.html 
 F2256901: report-a71e40.html 
 F2256903: report-245144.html 
 F2256902: report-cf6e19.html 


The hardest part would be, of course, dealing with `UnknownVal`s and 
`UndefinedVal`s in conditions, because they are completely ignored by these 
visitors - after all, no constraints are being added.

So we're far from consistency, just some improvements.


https://reviews.llvm.org/D23300



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

Re: [PATCH] D23198: clang-rename rename-all: support reading old/newname pairs from a YAML file

[Miklos Vajna via cfe-commits]

vmiklos added inline comments.


Comment at: clang-rename/tool/ClangRename.cpp:65
@@ +64,3 @@
+
+  RenameAllInfo() : Offset(0) {}
+};

omtcyfz wrote:
> AFAIK there's no need to do that, integer types are by default initialized 
> with 0, aren't they?
Are you sure? Here is a minimal version that shows what goes wrong when that's 
not initialized explicitly: http://pastebin.com/raw/2ZsUgWf6 The "Use of 
uninitialised value of size 8" goes away with an explicit initialization.


https://reviews.llvm.org/D23198



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

r278123 - [ASTMatchers] Add matchers canReferToDecl() and hasUnderlyingDecl()

[Martin Bohme via cfe-commits]

Author: mboehme
Date: Tue Aug  9 10:07:52 2016
New Revision: 278123

URL: http://llvm.org/viewvc/llvm-project?rev=278123&view=rev
Log:
[ASTMatchers] Add matchers canReferToDecl() and hasUnderlyingDecl()

Summary: Required for D0

Reviewers: sbenza, klimek, aaron.ballman, alexfh

Subscribers: alexfh, klimek, cfe-commits

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

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

Modified: cfe/trunk/docs/LibASTMatchersReference.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibASTMatchersReference.html?rev=278123&r1=278122&r2=278123&view=diff
==
--- cfe/trunk/docs/LibASTMatchersReference.html (original)
+++ cfe/trunk/docs/LibASTMatchersReference.html Tue Aug  9 10:07:52 2016
@@ -4970,6 +4970,19 @@ Usable as: MatcherNamedDecl>hasUnderlyingDeclMatcherNamedDecl>
 InnerMatcher
+Matches a 
NamedDecl whose underlying declaration matches the given
+matcher.
+
+Given
+  namespace N { template void f(T t); }
+  template  void g() { using N::f; f(T()); }
+unresolvedLookupExpr(hasAnyDeclaration(
+namedDecl(hasUnderlyingDecl(hasName("::N::f")
+  matches the use of f in g() .
+
+
+
 MatcherNestedNameSpecifierLoc>hasPrefixMatcherNestedNameSpecifierLoc>
 InnerMatcher
 Matches on the prefix of 
a NestedNameSpecifierLoc.
 
@@ -5057,6 +5070,23 @@ matches the [webView ...] message invoca
 
 
 
+MatcherOverloadExpr>hasAnyDeclarationMatcherDecl> 
InnerMatcher
+Matches an 
OverloadExpr if any of the declarations in the set of
+overloads matches the given matcher.
+
+Given
+  template  void foo(T);
+  template  void bar(T);
+  template  void baz(T t) {
+foo(t);
+bar(t);
+  }
+unresolvedLookupExpr(hasAnyDeclaration(
+functionTemplateDecl(hasName("foo"
+  matches foo in foo(t); but not bar in bar(t);
+
+
+
 MatcherParenType>innerTypeMatcherType>
 Matches ParenType nodes 
where the inner type is a specific type.
 

Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=278123&r1=278122&r2=278123&view=diff
==
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Tue Aug  9 10:07:52 2016
@@ -2468,6 +2468,25 @@ hasDeclaration(const internal::Matcher(InnerMatcher);
 }
 
+/// \brief Matches a \c NamedDecl whose underlying declaration matches the 
given
+/// matcher.
+///
+/// Given
+/// \code
+///   namespace N { template void f(T t); }
+///   template  void g() { using N::f; f(T()); }
+/// \endcode
+/// \c unresolvedLookupExpr(hasAnyDeclaration(
+/// namedDecl(hasUnderlyingDecl(hasName("::N::f")
+///   matches the use of \c f in \c g() .
+AST_MATCHER_P(NamedDecl, hasUnderlyingDecl, internal::Matcher,
+  InnerMatcher) {
+  const NamedDecl *UnderlyingDecl = Node.getUnderlyingDecl();
+
+  return UnderlyingDecl != nullptr &&
+ InnerMatcher.matches(*UnderlyingDecl, Finder, Builder);
+}
+
 /// \brief Matches on the implicit object argument of a member call expression.
 ///
 /// Example matches y.x()
@@ -2823,6 +2842,27 @@ AST_MATCHER_P(DeclRefExpr, throughUsingD
   return false;
 }
 
+/// \brief Matches an \c OverloadExpr if any of the declarations in the set of
+/// overloads matches the given matcher.
+///
+/// Given
+/// \code
+///   template  void foo(T);
+///   template  void bar(T);
+///   template  void baz(T t) {
+/// foo(t);
+/// bar(t);
+///   }
+/// \endcode
+/// unresolvedLookupExpr(hasAnyDeclaration(
+/// functionTemplateDecl(hasName("foo"
+///   matches \c foo in \c foo(t); but not \c bar in \c bar(t);
+AST_MATCHER_P(OverloadExpr, hasAnyDeclaration, internal::Matcher,
+  InnerMatcher) {
+  return matchesFirstInPointerRange(InnerMatcher, Node.decls_begin(),
+Node.decls_end(), Finder, Builder);
+}
+
 /// \brief Matches the Decl of a DeclStmt which has a single declaration.
 ///
 /// Given

Modified: cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
URL: 
http://llvm.org/viewvc/llvm-proj

Re: [PATCH] D23004: [ASTMatchers] Add matchers canReferToDecl() and hasUnderlyingDecl()

[Martin Böhme via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL278123: [ASTMatchers] Add matchers canReferToDecl() and 
hasUnderlyingDecl() (authored by mboehme).

Changed prior to commit:
  https://reviews.llvm.org/D23004?vs=66912&id=67353#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D23004

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

Index: cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
+++ cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -198,6 +198,7 @@
   REGISTER_MATCHER(hasAncestor);
   REGISTER_MATCHER(hasAnyArgument);
   REGISTER_MATCHER(hasAnyConstructorInitializer);
+  REGISTER_MATCHER(hasAnyDeclaration);
   REGISTER_MATCHER(hasAnyName);
   REGISTER_MATCHER(hasAnyParameter);
   REGISTER_MATCHER(hasAnySubstatement);
@@ -265,6 +266,7 @@
   REGISTER_MATCHER(hasTypeLoc);
   REGISTER_MATCHER(hasUnaryOperand);
   REGISTER_MATCHER(hasUnarySelector);
+  REGISTER_MATCHER(hasUnderlyingDecl);
   REGISTER_MATCHER(hasValueType);
   REGISTER_MATCHER(ifStmt);
   REGISTER_MATCHER(ignoringImplicit);
Index: cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -241,6 +241,21 @@
 hasDeclaration(namedDecl(hasName("A";
 }
 
+TEST(HasUnderlyingDecl, Matches) {
+  EXPECT_TRUE(matches("namespace N { template  void f(T t); }"
+  "template  void g() { using N::f; f(T()); }",
+  unresolvedLookupExpr(hasAnyDeclaration(
+  namedDecl(hasUnderlyingDecl(hasName("::N::f")));
+  EXPECT_TRUE(matches(
+  "namespace N { template  void f(T t); }"
+  "template  void g() { N::f(T()); }",
+  unresolvedLookupExpr(hasAnyDeclaration(namedDecl(hasName("::N::f"));
+  EXPECT_TRUE(notMatches(
+  "namespace N { template  void f(T t); }"
+  "template  void g() { using N::f; f(T()); }",
+  unresolvedLookupExpr(hasAnyDeclaration(namedDecl(hasName("::N::f"));
+}
+
 TEST(HasType, TakesQualTypeMatcherAndMatchesExpr) {
   TypeMatcher ClassX = hasDeclaration(recordDecl(hasName("X")));
   EXPECT_TRUE(
@@ -2072,5 +2087,24 @@
   EXPECT_TRUE(notMatches(Code2, ForEachOverriddenInClass("A1")));
 }
 
+TEST(Matcher, HasAnyDeclaration) {
+  std::string Fragment = "void foo(int p1);"
+ "void foo(int *p2);"
+ "void bar(int p3);"
+ "template  void baz(T t) { foo(t); }";
+
+  EXPECT_TRUE(
+  matches(Fragment, unresolvedLookupExpr(hasAnyDeclaration(functionDecl(
+hasParameter(0, parmVarDecl(hasName("p1";
+  EXPECT_TRUE(
+  matches(Fragment, unresolvedLookupExpr(hasAnyDeclaration(functionDecl(
+hasParameter(0, parmVarDecl(hasName("p2";
+  EXPECT_TRUE(
+  notMatches(Fragment, unresolvedLookupExpr(hasAnyDeclaration(functionDecl(
+   hasParameter(0, parmVarDecl(hasName("p3";
+  EXPECT_TRUE(notMatches(Fragment, unresolvedLookupExpr(hasAnyDeclaration(
+   functionDecl(hasName("bar"));
+}
+
 } // namespace ast_matchers
 } // namespace clang
Index: cfe/trunk/docs/LibASTMatchersReference.html
===
--- cfe/trunk/docs/LibASTMatchersReference.html
+++ cfe/trunk/docs/LibASTMatchersReference.html
@@ -4970,6 +4970,19 @@
 
 
 
+MatcherNamedDecl>hasUnderlyingDeclMatcherNamedDecl> InnerMatcher
+Matches a NamedDecl whose underlying declaration matches the given
+matcher.
+
+Given
+  namespace N { template void f(T t); }
+  template  void g() { using N::f; f(T()); }
+unresolvedLookupExpr(hasAnyDeclaration(
+namedDecl(hasUnderlyingDecl(hasName("::N::f")
+  matches the use of f in g() .
+
+
+
 MatcherNestedNameSpecifierLoc>hasPrefixMatcherNestedNameSpecifierLoc> InnerMatcher
 Matches on the prefix of a NestedNameSpecifierLoc.
 
@@ -5057,6 +5070,23 @@
 
 
 
+MatcherOverloadExpr>hasAnyDeclarationMatcherDecl> InnerMatcher
+Matches an OverloadExpr if any of the declarations in the set of
+overloads matches the given matcher.
+
+

Re: [PATCH] D23317: clang-format: Add SpaceAfterTemplate

[Michael Rybakov via cfe-commits]

opilarium added a comment.

Thanks a lot!


https://reviews.llvm.org/D23317



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

Re: [PATCH] D23265: [clang-tidy] enhance readability-else-after-return

[Alexander Kornienko via cfe-commits]

alexfh added inline comments.


Comment at: clang-tidy/readability/ElseAfterReturnCheck.cpp:45
@@ +44,3 @@
+  for (const auto &BindingName :
+   {"return", "continue", "break", "goto", "throw"}) {
+if (Result.Nodes.getNodeAs(BindingName)) {

This won't work in MSVC2013, I think. Just add a `const char *Labels[] = 
{"return", ...` (add more consts, constexprs or statics, if you like ;)


Comment at: clang-tidy/readability/ElseAfterReturnCheck.cpp:51
@@ +50,3 @@
+
+  DiagnosticBuilder Diag = diag(ElseLoc, "do not use 'else' after '" +
+ ControlFlowInterruptor + '\'');

Please use diagnostic formatting facilities instead of string concatenation:

  diag(L, "xxx '%0' yyy") << ControlFlowInterruptor;


Comment at: docs/clang-tidy/checks/readability-else-after-return.rst:10
@@ -6,1 +9,3 @@
+or `else if` after something that interrupts control flow - like `return`,
+`break`, `continue`, `throw`, `goto`, etc.
 

I would omit `goto`, since it's unclear, where the target label is (and it's 
not a common construct anyway).


Comment at: test/clang-tidy/readability-else-after-return.cpp:33
@@ -32,1 +32,3 @@
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use 'else' after 'return'
+  // CHECK-FIXES: // comment
 f(0);

CHECK-FIXES pattern should be sticter. Current pattern will match for any `// 
comment` occurrence anywhere in the test file, even if it's on a different line 
and still prefixed with `else`.

Two action items here: 1. make comments and patterns unique, 2. anchor patterns 
to the start of line (`{{ˆ}}`).


https://reviews.llvm.org/D23265



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

Re: [PATCH] D17990: [clang-tidy] minor improvements in modernise-deprecated-headers check

[Alexander Kornienko via cfe-commits]

alexfh requested changes to this revision.
This revision now requires changes to proceed.


Comment at: clang-tidy/modernize/DeprecatedHeadersCheck.cpp:55
@@ +54,3 @@
+  CStyledHeaderToCxx =
+   {{"assert.h", "cassert"},
+{"complex.h", "complex"},

It might not work on MSVC2013. You can try submitting this, but please watch 
the buildbots closely and have an alternative patch ready to fix the issue.


Comment at: clang-tidy/modernize/DeprecatedHeadersCheck.cpp:85
@@ -78,2 +84,3 @@
   }
+  DeleteHeaders = {"stdalign.h", "stdbool.h", "iso646.h"};
 }

ditto


Comment at: test/clang-tidy/modernize-deprecated-headers-cxx03.cpp:5
@@ -4,1 +4,3 @@
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ 
header 'assert.h'; consider using 'cassert' instead 
[modernize-deprecated-headers]
+// CHECK-FIXES: #include 
 #include 

Please anchor the patterns to the start and end of line.


Comment at: test/clang-tidy/modernize-deprecated-headers-cxx03.cpp:60
@@ -27,2 +59,3 @@
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: including 'stdalign.h' has no 
effect in C++; consider removing it
 #include 

Please add a comment after the #include and add a CHECK-FIXES to ensure the 
whole #include directive is removed.


Comment at: test/clang-tidy/modernize-deprecated-headers-cxx03.cpp:131
@@ +130,3 @@
+#include "stdalign.h"
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: including 'stdalign.h' has no 
effect in C++; consider removing it
+#include "stdbool.h"

Add proper CHECK-FIXES here as well.

The same for the other test file.


https://reviews.llvm.org/D17990



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

[PATCH] D23322: [OpenCL] AMDGPU: Add extensions cl_amd_media_ops and cl_amd_media_ops2

[Yaxun Liu via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: nhaustov, Anastasia.
yaxunl added subscribers: cfe-commits, tstellarAMD.
Herald added a reviewer: tstellarAMD.
Herald added a subscriber: kzhuravl.

https://reviews.llvm.org/D23322

Files:
  include/clang/Basic/OpenCLExtensions.def
  lib/Basic/Targets.cpp
  lib/Headers/opencl-c.h
  test/Misc/amdgcn.languageOptsOpenCL.cl
  test/SemaOpenCL/extension-version.cl

Index: test/SemaOpenCL/extension-version.cl
===
--- test/SemaOpenCL/extension-version.cl
+++ test/SemaOpenCL/extension-version.cl
@@ -259,3 +259,14 @@
 // expected-warning@+2{{unsupported OpenCL extension 'cl_khr_terminate_context' - ignoring}}
 #endif
 #pragma OPENCL EXTENSION cl_khr_terminate_context: enable
+
+#ifndef cl_amd_media_ops
+#error "Missing cl_amd_media_ops define"
+#endif
+#pragma OPENCL EXTENSION cl_amd_media_ops: enable
+
+#ifndef cl_amd_media_ops2
+#error "Missing cl_amd_media_ops2 define"
+#endif
+#pragma OPENCL EXTENSION cl_amd_media_ops2: enable
+
Index: test/Misc/amdgcn.languageOptsOpenCL.cl
===
--- test/Misc/amdgcn.languageOptsOpenCL.cl
+++ test/Misc/amdgcn.languageOptsOpenCL.cl
@@ -210,3 +210,14 @@
 #endif
 #pragma OPENCL EXTENSION cl_khr_terminate_context: enable
 // expected-warning@-1{{unsupported OpenCL extension 'cl_khr_terminate_context' - ignoring}}
+
+#ifndef cl_amd_media_ops
+#error "Missing cl_amd_media_ops define"
+#endif
+#pragma OPENCL EXTENSION cl_amd_media_ops: enable
+
+#ifndef cl_amd_media_ops2
+#error "Missing cl_amd_media_ops2 define"
+#endif
+#pragma OPENCL EXTENSION cl_amd_media_ops2: enable
+
Index: lib/Headers/opencl-c.h
===
--- lib/Headers/opencl-c.h
+++ lib/Headers/opencl-c.h
@@ -16843,6 +16843,196 @@
 
 #endif //cl_khr_subgroups cl_intel_subgroups
 
+#ifdef cl_amd_media_ops
+uint __ovld amd_bitalign(uint a, uint b, uint c);
+uint2 __ovld amd_bitalign(uint2 a, uint2 b, uint2 c);
+uint3 __ovld amd_bitalign(uint3 a, uint3 b, uint3 c);
+uint4 __ovld amd_bitalign(uint4 a, uint4 b, uint4 c);
+uint8 __ovld amd_bitalign(uint8 a, uint8 b, uint8 c);
+uint16 __ovld amd_bitalign(uint16 a, uint16 b, uint16 c);
+
+uint __ovld amd_bytealign(uint a, uint b, uint c);
+uint2 __ovld amd_bytealign(uint2 a, uint2 b, uint2 c);
+uint3 __ovld amd_bytealign(uint3 a, uint3 b, uint3 c);
+uint4 __ovld amd_bytealign(uint4 a, uint4 b, uint4 c);
+uint8 __ovld amd_bytealign(uint8 a, uint8 b, uint8 c);
+uint16 __ovld amd_bytealign(uint16 a, uint16 b, uint16 c);
+
+uint __ovld amd_lerp(uint a, uint b, uint c);
+uint2 __ovld amd_lerp(uint2 a, uint2 b, uint2 c);
+uint3 __ovld amd_lerp(uint3 a, uint3 b, uint3 c);
+uint4 __ovld amd_lerp(uint4 a, uint4 b, uint4 c);
+uint8 __ovld amd_lerp(uint8 a, uint8 b, uint8 c);
+uint16 __ovld amd_lerp(uint16 a, uint16 b, uint16 c);
+
+uint __ovld amd_pack(float4 v);
+
+uint __ovld amd_sad4(uint4 x, uint4 y, uint z);
+
+uint __ovld amd_sadhi(uint a, uint b, uint c);
+uint2 __ovld amd_sadhi(uint2 a, uint2 b, uint2 c);
+uint3 __ovld amd_sadhi(uint3 a, uint3 b, uint3 c);
+uint4 __ovld amd_sadhi(uint4 a, uint4 b, uint4 c);
+uint8 __ovld amd_sadhi(uint8 a, uint8 b, uint8 c);
+uint16 __ovld amd_sadhi(uint16 a, uint16 b, uint16 c);
+
+uint __ovld amd_sad(uint a, uint b, uint c);
+uint2 __ovld amd_sad(uint2 a, uint2 b, uint2 c);
+uint3 __ovld amd_sad(uint3 a, uint3 b, uint3 c);
+uint4 __ovld amd_sad(uint4 a, uint4 b, uint4 c);
+uint8 __ovld amd_sad(uint8 a, uint8 b, uint8 c);
+uint16 __ovld amd_sad(uint16 a, uint16 b, uint16 c);
+
+float __ovld amd_unpack0(uint a);
+float2 __ovld amd_unpack0(uint2 a);
+float3 __ovld amd_unpack0(uint3 a);
+float4 __ovld amd_unpack0(uint4 a);
+float8 __ovld amd_unpack0(uint8 a);
+float16 __ovld amd_unpack0(uint16 a);
+
+float __ovld amd_unpack1(uint a);
+float2 __ovld amd_unpack1(uint2 a);
+float3 __ovld amd_unpack1(uint3 a);
+float4 __ovld amd_unpack1(uint4 a);
+float8 __ovld amd_unpack1(uint8 a);
+float16 __ovld amd_unpack1(uint16 a);
+
+float __ovld amd_unpack2(uint a);
+float2 __ovld amd_unpack2(uint2 a);
+float3 __ovld amd_unpack2(uint3 a);
+float4 __ovld amd_unpack2(uint4 a);
+float8 __ovld amd_unpack2(uint8 a);
+float16 __ovld amd_unpack2(uint16 a);
+
+float __ovld amd_unpack3(uint a);
+float2 __ovld amd_unpack3(uint2 a);
+float3 __ovld amd_unpack3(uint3 a);
+float4 __ovld amd_unpack3(uint4 a);
+float8 __ovld amd_unpack3(uint8 a);
+float16 __ovld amd_unpack3(uint16 a);
+#endif // cl_amd_media_ops
+
+#ifdef cl_amd_media_ops2
+int __ovld amd_bfe(int src0, uint src1, uint src2);
+int2 __ovld amd_bfe(int2 src0, uint2 src1, uint2 src2);
+int3 __ovld amd_bfe(int3 src0, uint3 src1, uint3 src2);
+int4 __ovld amd_bfe(int4 src0, uint4 src1, uint4 src2);
+int8 __ovld amd_bfe(int8 src0, uint8 src1, uint8 src2);
+int16 __ovld amd_bfe(int16 src0, uint16 src1, uint16 src2);
+
+uint __ovld amd_bfe(uint src0, uint src1, uint src2);
+uint2 __ovld amd

Re: [PATCH] D23320: [analyzer] Fixed crash in CloneDetector in function calls.

[Vassil Vassilev via cfe-commits]

v.g.vassilev accepted this revision.
v.g.vassilev added a comment.

LGTM.


https://reviews.llvm.org/D23320



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

Re: [PATCH] D23317: clang-format: Add SpaceAfterTemplate

[Daniel Jasper via cfe-commits]

djasper added a comment.

I don't think this is worth a release note.


https://reviews.llvm.org/D23317



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

Re: [PATCH] D23279: clang-reorder-fields

[Kirill Bobyrev via cfe-commits]

omtcyfz added a comment.

In https://reviews.llvm.org/D23279#509896, @compnerd wrote:

> If the argument really is that we want to minimize the tools then Id argue 
> that `clang-rename` also belongs in `clang-tidy` as it would be used to 
> rename fields to match the naming convention (tidying up your code base).


It does not belong to `clang-tidy`. `clang-tidy` is a linter, it is meant for 
diagnosing and fixing typical programming errors.

> `clang-tidy` could work, but it does seem to be introducing a completely new 
> concept into `clang-tidy` AFAICT.  It has so far only done equivalent 
> changes.  This operation doesn't guarantee equivalence: if you are doing a 
> `reinterpret_cast` or a C-style cast, that will no longer work as the object 
> layout has changed.

> 

> We could merge both tools into `clang-tidy`, or perhaps we can hold off on 
> that for the wider discussion, and allow this to make progress in the mean 
> time.



Repository:
  rL LLVM

https://reviews.llvm.org/D23279



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

Re: [PATCH] D23279: clang-reorder-fields

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D23279#510002, @omtcyfz wrote:

> In https://reviews.llvm.org/D23279#509896, @compnerd wrote:
>
> > If the argument really is that we want to minimize the tools then Id argue 
> > that `clang-rename` also belongs in `clang-tidy` as it would be used to 
> > rename fields to match the naming convention (tidying up your code base).
>
>
> It does not belong to `clang-tidy`. `clang-tidy` is a linter, it is meant for 
> diagnosing and fixing typical programming errors.


We have the modernize and readability modules which don't really deal with 
programming errors at all (for some definition of programming error).


Repository:
  rL LLVM

https://reviews.llvm.org/D23279



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

Re: [PATCH] D23125: Modules: add command line option to support loading prebuilt modules on demand, without parsing any module map

[Ben Langmuir via cfe-commits]

benlangmuir added a comment.

Sorry for the delay, I apparently forgot to hit submit. Replied inline.



Comment at: lib/Frontend/CompilerInstance.cpp:1491
@@ +1490,3 @@
+false/*IsExplicit*/).first;
+Module->IsSystem = true;
+Module->IsFromModuleFile = true;

manmanren wrote:
> benlangmuir wrote:
> > Why do we assume this?
> Thanks for reviewing so quickly!
> 
> I was not sure about what value we should set IsSystem. Since we are mostly 
> prebuilding system modules, I assumed "true". Do you have any suggestion here?
I guess treating it as system is the more forgiving option, so I'm okay with 
keeping it.  It's probably worth adding a comment that this is not always the 
correct choice, since it can affect diagnostics, etc.


https://reviews.llvm.org/D23125



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

[PATCH] D23325: [WIP] Binding of references to packed fields

[Roger Ferrer Ibanez via cfe-commits]

rogfer01 created this revision.
rogfer01 added reviewers: aaron.ballman, rsmith.
rogfer01 added subscribers: cfe-commits, rsmith.

This is a WIP for PR28571.

As suggested by @rsmith it introduces a new ObjectKind for PackedField's 
modeled similarly in spirit to BitField's. If the reference is const, the 
binding is done using a temporary otherwise an error is diagnosed.

https://reviews.llvm.org/D23325

Files:
  include/clang/AST/Decl.h
  include/clang/AST/Expr.h
  include/clang/AST/Stmt.h
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/Specifiers.h
  lib/AST/ASTDumper.cpp
  lib/AST/Decl.cpp
  lib/CodeGen/CGCall.cpp
  lib/Sema/SemaCast.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaExprMember.cpp
  lib/Sema/SemaFixItUtils.cpp
  lib/Sema/SemaInit.cpp
  test/SemaCXX/bind-packed-member.cpp

Index: test/SemaCXX/bind-packed-member.cpp
===
--- /dev/null
+++ test/SemaCXX/bind-packed-member.cpp
@@ -0,0 +1,87 @@
+// RUN: %clang_cc1 -verify %s
+
+struct __attribute__((packed)) A {
+  char x;
+  float y;
+  char z;
+} a;
+
+char &rax = a.x;  // no-error
+float &ray = a.y; // expected-error {{reference cannot bind to packed field}}
+char &raz = a.z;  // no-error
+
+struct __attribute__((packed, aligned(2))) B {
+  // Regardless of aligned(2) the fields are aligned(1) because of packed.
+  // The whole struct is aligned(2), though.
+  short x;
+  float y;
+  short z;
+  char w;
+} b;
+
+const short &crbx = b.x; // no-error
+short &rbx = b.x; // expected-error {{reference cannot bind to packed field}}
+float &rby = b.y; // expected-error {{reference cannot bind to packed field}}
+short &rbz = b.z; // expected-error {{reference cannot bind to packed field}}
+char &rbw = b.w;  // no-error
+
+struct __attribute__((packed)) B2 {
+  short x __attribute__((aligned(2)));
+  float y;
+  short z __attribute__((aligned(4)));
+  char w;
+} b2;
+
+short &rb2x = b2.x; // no-error
+short &rb2z = b2.z; // no-error
+
+struct C {
+  int c;
+};
+
+struct __attribute__((packed)) D {
+  char x;
+  int y;
+  C z;
+} d;
+
+C &rcz = d.z; // expected-error {{reference cannot bind to packed field}}
+int &rczc = d.z.c; // expected-error {{reference cannot bind to packed field}}
+
+struct E {
+int x __attribute__((packed));
+C y __attribute__((packed));
+C z;
+} e;
+
+int& rex = e.x; // expected-error {{reference cannot bind to packed field}}
+C& rey = e.y; // expected-error {{reference cannot bind to packed field}}
+C& rez = e.z;  // no-error
+
+struct NonTrivialCopy
+{
+int w;
+NonTrivialCopy();
+NonTrivialCopy(const NonTrivialCopy&);
+};
+
+struct F
+{
+char c;
+NonTrivialCopy x __attribute__((packed));
+int w __attribute__((packed));
+} f;
+
+
+void fun1(int &);
+void fun2(const int &);
+
+void bar()
+{
+const NonTrivialCopy& rx = f.x; // expected-error {{reference cannot bind to packed field}}
+const int &w = f.w; // no-error
+
+fun1(f.w); // expected-error {{reference cannot bind to packed field}}
+   // expected-note@76 {{passing argument to parameter here}}
+fun2(f.w); // no-error
+}
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -4168,6 +4168,7 @@
  Qualifiers T2Quals,
  bool IsLValueRef) {
   bool IsNonAddressableType = Initializer->refersToBitField() ||
+  Initializer->refersToPackedField() ||
   Initializer->refersToVectorElement();
 
   if (IsNonAddressableType) {
@@ -4182,6 +4183,30 @@
   return Initializer->getValueKind();
 }
 
+if (IsLValueRef && Initializer->refersToPackedField() &&
+Initializer->getType()->getAsCXXRecordDecl()) {
+  auto Record = Initializer->getType()->getAsCXXRecordDecl();
+  /*
+Consider the case below:
+   struct A
+   {
+   char c;
+   std::vector x __attribute__((packed));
+   } a;
+
+   void f()
+   {
+   const std::vector& w = a.x;
+   }
+
+   It is not possible to bind w to a temporary of a.x because this
+   would require (recursively) invoking the copy constructor of
+   std::vector to obtain first a (properly aligned) temporary of a.x.
+  */
+  if (!Record->hasTrivialCopyConstructor())
+return Initializer->getValueKind();
+}
+
 // Force a load so we can materialize a temporary.
 Sequence.AddLValueToRValueStep(cv1T1.getUnqualifiedType());
 return VK_RValue;
@@ -6431,6 +6456,13 @@
 return ExprError();
   }
 
+  if (CurInit.get()->refersToPackedField()) {
+S.Diag(Kind.getLocation(), diag::err_reference_bind_to_packed_field)
+<< CurInit.get()->getSourceRange();
+PrintInitLocationNote(S, Entity);
+return ExprError(

Re: [PATCH] D23325: [WIP] Binding of references to packed fields

[Roger Ferrer Ibanez via cfe-commits]

rogfer01 added inline comments.


Comment at: lib/Sema/SemaInit.cpp:4202-4204
@@ +4201,5 @@
+
+   It is not possible to bind w to a temporary of a.x because this
+   would require (recursively) invoking the copy constructor of
+   std::vector to obtain first a (properly aligned) temporary of a.x.
+  */

Rereading this makes no sense to me now. I'll adress this in a later update.


https://reviews.llvm.org/D23325



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

Re: [PATCH] D23279: clang-reorder-fields

[Kirill Bobyrev via cfe-commits]

omtcyfz added a comment.

In https://reviews.llvm.org/D23279#510011, @aaron.ballman wrote:

> In https://reviews.llvm.org/D23279#510002, @omtcyfz wrote:
>
> > In https://reviews.llvm.org/D23279#509896, @compnerd wrote:
> >
> > > If the argument really is that we want to minimize the tools then Id 
> > > argue that `clang-rename` also belongs in `clang-tidy` as it would be 
> > > used to rename fields to match the naming convention (tidying up your 
> > > code base).
> >
> >
> > It does not belong to `clang-tidy`. `clang-tidy` is a linter, it is meant 
> > for diagnosing and fixing typical programming errors.
>
>
> We have the modernize and readability modules which don't really deal with 
> programming errors at all (for some definition of programming error).


All of the checks in `clang-tidy` **issues**. These are also issues:

- code style violations
- inefficiency

And other things.

There is a clear difference between fixing issues and refactoring. 
`clang-format` does refactoring, `clang-tidy` deals with issues.


Repository:
  rL LLVM

https://reviews.llvm.org/D23279



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

Re: r266719 - Warn if function or variable cannot be implicitly instantiated

[Serge Pavlov via cfe-commits]

Whether enable this warning or not should be determined by user feedback.
The warning was implemented to help users in complicated cases that arise
in module-enabled builds. It seems however that it makes troubles for other
users. Based on feedback on this list I feel that it is better to make this
message off by default. It still should be useful for people who use
explicit instantiation, but this use case is rare, I think.

Should someone approve turning the message off by default, or discussion
here is enough?


Thanks,
--Serge

2016-08-09 20:44 GMT+07:00 Nico Weber :

> With 3.9 coming around, I'd like to suggest that we pull this warning from
> 3.9 and maybe trunk. It sounds like Sean found it only possible to deploy
> this warning since they already had a workaround for a different compiler
> bug (!). In Chromium, we can't enable this warning since one of our
> (admittedly dubiously designed) template classes in a foundational library
> requires people to have an explicit instantiation of their downstream
> classes in a client cc file. Fixing this warning would mean giving the h
> file in the foundational library forward declarations of all clients of the
> template.
>
> The motivation for this warning was PR24425, which is something that's
> only relevant with modules enabled. Maybe the warning should only fire with
> modules. But as-is, the warning warns about something that isn't really a
> problem in practice, and it's difficult to fix (and as said, fixing it has
> few benefits). I don't think it's at the bar we have for clang warnings.
>
> Is there anyone who has deployed this warning successfully on a larger
> codebase? Examples of real bugs it found?
>
> On Fri, May 20, 2016 at 12:14 AM, Sean Silva 
> wrote:
>
>>
>>
>> On Thu, May 19, 2016 at 12:13 PM, Serge Pavlov 
>> wrote:
>>
>>> In this case moving implementation of `Singleton::getInstance()` into
>>> a .cpp file would prevent compiler from instantiation of the method body
>>> when it sees `Singleton::getInstance()`. In this case
>>> `Singleton::getInstance()` must be instantiated in some source
>>> file either explicitly or implicitly. If inline implementation for
>>> `Singleton::getInstance()` is provided in the header, where
>>> `Singleton` is defined, the method body is instantiated implicitly when
>>> it is required. But the static field `instance` referenced in the method
>>> still must be instantiated in some source file explicitly or implicitly. So
>>> from viewpoint of convenience, moving the implementation of template
>>> `Singleton::getInstance()` into source file does not look as more
>>> inflexible solution.
>>>
>>> Probably it is more convenient to put the template for the static member
>>> to header file too:
>>>
>>> template 
>>> T *Singleton::instance = nullptr;
>>>
>>>
>>> In this case both the method and corresponding static member are
>>> instantiated implicitly by compiler, no warning occurs.
>>> Can it be an acceptable solution?
>>>
>>
>> I think that is what makes the most sense in this scenario (and it
>> simplifies things for clients of the library).
>> Unfortunately, for the library that was producing this warning they
>> already required clients to provide explicit instantiations in a .cpp file
>> (they had a macro like `DEFINE_SINGLETON_INSTANCE(T)` which a user would
>> place in a .cpp file to instantiate Singleton::instance for their type).
>>
>> So fixing this warning like this would have a compatibility concern.
>>
>> Thankfully, it seems like some compilers (not clang) have a bug in that
>> they will emit Singleton::instance any time that Singleton is
>> instantiated unless they have already seen an explicit instantiation
>> declaration of Singleton::instance in a header, so this library
>> already had (under an #if) explicit instantiations for
>> Singleton::instance for all classes that needed it in order to work
>> around this compiler bug. So in the end I fixed this by just enabling those
>> ifdefs so that clang would see the Singleton::instance explicitly
>> declared in the header.
>> (This is sort of a strange coincidence, but it worked out nicely)
>>
>>
>>
>>>
>>> If there is intention to limit `Singleton` to some selected types,
>>> explicit instantiation declaration may help. Adding the declarations like:
>>>
>>> extern template Foo *Singleton::instance;
>>>
>>>
>>> prevents from warnings.
>>>
>>> Or you think that the message should propose moving static member
>>> definition into header file as well?
>>>
>>
>> I'm not sure, but I think that the current diagnostics are not very
>> actionable. Hopefully google will lead users to your description in this
>> thread. I wish we had something like https://doc.rust-lang.org
>> /error-index.html where we could officially provide a more in-depth
>> explanation similar to your posts in this thread.
>>
>> -- Sean Silva
>>
>>
>>>
>>>
>>> Thanks,
>>> --Serge
>>>
>>> 2016-05-19 9:15 GMT+06:00 Sean Silva :
>>>


 On Thu, Apr 21, 2016 at 12:44 AM, Serg

Re: [PATCH] D23265: [clang-tidy] enhance readability-else-after-return

[Alexander Kornienko via cfe-commits]

alexfh requested changes to this revision.
This revision now requires changes to proceed.


Comment at: clang-tidy/readability/ElseAfterReturnCheck.cpp:25
@@ +24,3 @@
+  forEach(ifStmt(hasThen(stmt(anyOf(
+ returnStmt().bind("return"),
+ compoundStmt(has(returnStmt().bind("return"))),

Let's structure this differently to reduce code duplication:

  auto returnLikeStmt = stmt(anyOf(returnStmt().bind("return"), 
continueStmt().bind("continue"), ...));
  Finder->addMatcher(stmt(forEach(ifStmt(hasThen(anyOf(returnLikeStmt, 
compoundStmt(has(returnLikeStmt, hasElse(...


https://reviews.llvm.org/D23265



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

Re: [PATCH] D23279: clang-reorder-fields

[Aaron Ballman via cfe-commits]

On Tue, Aug 9, 2016 at 12:14 PM, Kirill Bobyrev
 wrote:
> omtcyfz added a comment.
>
> In https://reviews.llvm.org/D23279#510011, @aaron.ballman wrote:
>
>> In https://reviews.llvm.org/D23279#510002, @omtcyfz wrote:
>>
>> > In https://reviews.llvm.org/D23279#509896, @compnerd wrote:
>> >
>> > > If the argument really is that we want to minimize the tools then Id 
>> > > argue that `clang-rename` also belongs in `clang-tidy` as it would be 
>> > > used to rename fields to match the naming convention (tidying up your 
>> > > code base).
>> >
>> >
>> > It does not belong to `clang-tidy`. `clang-tidy` is a linter, it is meant 
>> > for diagnosing and fixing typical programming errors.
>>
>>
>> We have the modernize and readability modules which don't really deal with 
>> programming errors at all (for some definition of programming error).
>
>
> All of the checks in `clang-tidy` **issues**. These are also issues:
>
> - code style violations
> - inefficiency
>
> And other things.
>
> There is a clear difference between fixing issues and refactoring. 
> `clang-format` does refactoring, `clang-tidy` deals with issues.

Consider modernize-loop-convert, modernize-raw-string-literal, and
modernize-use-override. These are most definitely refactoring tools
that exist within clang-tidy and they do not point out issues with the
user's code. That being said, I'm opposed to the proliferation of
stand-alone tools when possible. clang-format does not strike me as a
good home for the proposed functionality because reformatting does not
suggest semantic modifications to the user's source code, and it
certainly would not be the first tool I would think to reach for to
get this functionality. I think clang-tidy is a more reasonable tool
for it. Ideally, we would have a refactoring module that houses most
(if not all) of modernize's functionality + refactoring passes that
are not really modernizations (such as the recent review of converting
memcpy() -> std::copy()).

>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D23279
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D23198: clang-rename rename-all: support reading old/newname pairs from a YAML file

[Kirill Bobyrev via cfe-commits]

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

LGTM



Comment at: clang-rename/tool/ClangRename.cpp:65
@@ +64,3 @@
+
+  RenameAllInfo() : Offset(0) {}
+};

vmiklos wrote:
> omtcyfz wrote:
> > AFAIK there's no need to do that, integer types are by default initialized 
> > with 0, aren't they?
> Are you sure? Here is a minimal version that shows what goes wrong when 
> that's not initialized explicitly: http://pastebin.com/raw/2ZsUgWf6 The "Use 
> of uninitialised value of size 8" goes away with an explicit initialization.
Well, in this case - yes. But in that case the default unsigned constructor 
isn't called.

What I meant is:

```
RenameAllInfo() : Offset(0) {}
```
- > 
```
RenameAllInfo() : Offset() {}
```

In this case Offset is default-constructed. However, it is also true that one 
might argue that it's less "trivial" (whatever "triviality" definition would be 
:]). I guess it's fine to leave it like this.


https://reviews.llvm.org/D23198



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

Re: [PATCH] D23198: clang-rename rename-all: support reading old/newname pairs from a YAML file

[Kirill Bobyrev via cfe-commits]

omtcyfz added inline comments.


Comment at: clang-rename/tool/ClangRename.cpp:65
@@ +64,3 @@
+
+  RenameAllInfo() : Offset(0) {}
+};

omtcyfz wrote:
> vmiklos wrote:
> > omtcyfz wrote:
> > > AFAIK there's no need to do that, integer types are by default 
> > > initialized with 0, aren't they?
> > Are you sure? Here is a minimal version that shows what goes wrong when 
> > that's not initialized explicitly: http://pastebin.com/raw/2ZsUgWf6 The 
> > "Use of uninitialised value of size 8" goes away with an explicit 
> > initialization.
> Well, in this case - yes. But in that case the default unsigned constructor 
> isn't called.
> 
> What I meant is:
> 
> ```
> RenameAllInfo() : Offset(0) {}
> ```
> - > 
> ```
> RenameAllInfo() : Offset() {}
> ```
> 
> In this case Offset is default-constructed. However, it is also true that one 
> might argue that it's less "trivial" (whatever "triviality" definition would 
> be :]). I guess it's fine to leave it like this.
> Well, in this case - yes. But in that case the default unsigned constructor 
> isn't called.



-> Well, in the case you sent - yes. But there is no default-construction there 
as there is no constructor with initializer list at all.


https://reviews.llvm.org/D23198



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

Re: [PATCH] D23279: clang-reorder-fields

[Alexander Shaposhnikov via cfe-commits]

alexshap added a comment.

First of all, many thanks for the comments & proposal of clang-refactor.

1. Regarding high-level project organization:

clang-refactor - that can be a good place for various refactoring techniques
like the existing clang-rename, my simple tool, etc - essentially what Kirill 
said in the e-mail.
I would be happy to join. 
At the same time - if i can make some progress before this reorganization is 
introduced - 
that would be helpful, if not - no problem at all.

2. Regarding clang-tidy - it seems to me, that putting this particular tool 
into PaddingChecker/clang-tidy is far from being perfect: A. In some cases the 
optimal fields order is not unique, it would be more flexible to let the user 
choose which order of fields he prefers. B. In some cases someone might want to 
change the order of fields even if the padding is not affected at all. C. Other 
concerns mentioned above in your comments

One more thing: 
i am not sure i understood the comment  
"It actually breaks code ... only works while it is in the same TU".
For instance if we have the following project:
/include/Point.h (contains the definition of the struct Point)
/a.cpp (uses Point)
/b.cpp (also uses Point)
/main.cpp (also uses Point)
clang-reorder-fields -i -record-name ::Point -fields-order y,x main.cpp a.cpp 
b.cpp
works for me.
At the same time i see in my code the following issue (easy to fix):
it will complain here :

  if (auto Err = Replacements[R.getFilePath()].add(R))
 llvm::errs() << "Failed to add replacement, file: " << R.getFilePath()

because the same replacement for the code in header is being added multiple 
times, 
but actually it doesn't affect the final set of changes and the result is 
correct (i think we need either to handle this case more carefully or 
(temporarily) add FIXME and replace llvm::errs with llvm::warns ).


Repository:
  rL LLVM

https://reviews.llvm.org/D23279



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

Re: [PATCH] D23239: [CUDA] Add __device__ overloads for placement new and delete.

[Artem Belevich via cfe-commits]

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

LGTM.


https://reviews.llvm.org/D23239



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

Re: [PATCH] D22946: [CUDA] Regression test to make sure C++ include path are forwarded to host and device frontends.

[Artem Belevich via cfe-commits]

tra added inline comments.


Comment at: test/Driver/cuda-detect.cu:75-77
@@ -74,1 +74,5 @@
 
+// Verify that compiler accepts CUDA syntax with "-x cuda-cpp-output".
+// RUN: %clang -Werror -fsyntax-only -x cuda-cpp-output -c %s
+//
+// Verify that C++ include paths are passed for both host and device frontends.

These two lines seem to be copy-pasted from cuda-simple.cu by mistake.


https://reviews.llvm.org/D22946



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

Re: [PATCH] D23279: clang-reorder-fields

[Ben Craig via cfe-commits]

bcraig added a subscriber: bcraig.
bcraig added a comment.

In https://reviews.llvm.org/D23279#509017, @Eugene.Zelenko wrote:

> Do we really need standalone tool for this purpose? If I'm not mistaken, 
> Static Analyzer already has clang-analyzer-optin.performance.Padding check, 
> which is also available through Clang-tidy.


The excess padding checker doesn't provide a fix-it.


Repository:
  rL LLVM

https://reviews.llvm.org/D23279



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

Re: [PATCH] D23279: clang-reorder-fields

[Ben Craig via cfe-commits]

bcraig added a comment.

It is very common for developers to put comments near a field.  If this tool 
doesn't move the "anchored" comments, then its utility will be severely limited.

Does this tool also reorder C++ member initializer lists?  If it doesn't, then 
running this tool will introduce warnings in code.

The static analyzer does have code to find a less padded ordering for a 
structure (lib/StaticAnalyzer/Checkers/PaddingChecker.cpp, 
calculateOptimalPad()).  You may want to consider using / factoring out that 
code for this tool.  It does have substantial limitations on which kinds of 
records it can look at though.


Repository:
  rL LLVM

https://reviews.llvm.org/D23279



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

Re: [PATCH] D23279: clang-reorder-fields

[Alexander Shaposhnikov via cfe-commits]

alexshap added a comment.

> The excess padding checker doesn't provide a fix-it.


yup, although i think it would be useful to add one of the optimal layouts to 
the report
(i have a diff for that (still in the oven), i have not sent it for code review 
yet).
But as i mentioned above - it seems to me that the proposed tool is more 
generic and binding it to the PaddingChecker
is not the best possible approach.


Repository:
  rL LLVM

https://reviews.llvm.org/D23279



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

Re: [PATCH] D22946: [CUDA] Regression test to make sure C++ include path are forwarded to host and device frontends.

[Samuel Antao via cfe-commits]

sfantao updated this revision to Diff 67366.
sfantao marked an inline comment as done.
sfantao added a comment.

- Remove redundant test - copied by mistake.


https://reviews.llvm.org/D22946

Files:
  test/Driver/cuda-detect.cu

Index: test/Driver/cuda-detect.cu
===
--- test/Driver/cuda-detect.cu
+++ test/Driver/cuda-detect.cu
@@ -72,6 +72,11 @@
 // RUN:   | FileCheck %s -check-prefix COMMON \
 // RUN: -check-prefix NOCUDAINC -check-prefix NOLIBDEVICE
 
+// Verify that C++ include paths are passed for both host and device frontends.
+// RUN: %clang -### -target x86_64-linux-gnu %s \
+// RUN: --sysroot=%S/Inputs/ubuntu_14.04_multiarch_tree2 2>&1 \
+// RUN: | FileCheck %s --check-prefix CHECK-CXXINCLUDE
+
 // CHECK: Found CUDA installation: {{.*}}/Inputs/CUDA/usr/local/cuda
 // NOCUDA-NOT: Found CUDA installation:
 
@@ -92,3 +97,8 @@
 // CUDAINC-SAME: "-include" "__clang_cuda_runtime_wrapper.h"
 // NOCUDAINC-NOT: "-include" "__clang_cuda_runtime_wrapper.h"
 // COMMON-SAME: "-x" "cuda"
+// CHECK-CXXINCLUDE: clang{{.*}} "-cc1" "-triple" "nvptx64-nvidia-cuda"
+// CHECK-CXXINCLUDE-SAME: {{.*}}"-internal-isystem" "{{.+}}/include/c++/4.8"
+// CHECK-CXXINCLUDE: clang{{.*}} "-cc1" "-triple" "x86_64--linux-gnu"
+// CHECK-CXXINCLUDE-SAME: {{.*}}"-internal-isystem" "{{.+}}/include/c++/4.8"
+// CHECK-CXXINCLUDE: ld{{.*}}"


Index: test/Driver/cuda-detect.cu
===
--- test/Driver/cuda-detect.cu
+++ test/Driver/cuda-detect.cu
@@ -72,6 +72,11 @@
 // RUN:   | FileCheck %s -check-prefix COMMON \
 // RUN: -check-prefix NOCUDAINC -check-prefix NOLIBDEVICE
 
+// Verify that C++ include paths are passed for both host and device frontends.
+// RUN: %clang -### -target x86_64-linux-gnu %s \
+// RUN: --sysroot=%S/Inputs/ubuntu_14.04_multiarch_tree2 2>&1 \
+// RUN: | FileCheck %s --check-prefix CHECK-CXXINCLUDE
+
 // CHECK: Found CUDA installation: {{.*}}/Inputs/CUDA/usr/local/cuda
 // NOCUDA-NOT: Found CUDA installation:
 
@@ -92,3 +97,8 @@
 // CUDAINC-SAME: "-include" "__clang_cuda_runtime_wrapper.h"
 // NOCUDAINC-NOT: "-include" "__clang_cuda_runtime_wrapper.h"
 // COMMON-SAME: "-x" "cuda"
+// CHECK-CXXINCLUDE: clang{{.*}} "-cc1" "-triple" "nvptx64-nvidia-cuda"
+// CHECK-CXXINCLUDE-SAME: {{.*}}"-internal-isystem" "{{.+}}/include/c++/4.8"
+// CHECK-CXXINCLUDE: clang{{.*}} "-cc1" "-triple" "x86_64--linux-gnu"
+// CHECK-CXXINCLUDE-SAME: {{.*}}"-internal-isystem" "{{.+}}/include/c++/4.8"
+// CHECK-CXXINCLUDE: ld{{.*}}"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22946: [CUDA] Regression test to make sure C++ include path are forwarded to host and device frontends.

[Samuel Antao via cfe-commits]

sfantao added inline comments.


Comment at: test/Driver/cuda-detect.cu:75-77
@@ -74,1 +74,5 @@
 
+// Verify that compiler accepts CUDA syntax with "-x cuda-cpp-output".
+// RUN: %clang -Werror -fsyntax-only -x cuda-cpp-output -c %s
+//
+// Verify that C++ include paths are passed for both host and device frontends.

tra wrote:
> These two lines seem to be copy-pasted from cuda-simple.cu by mistake.
Woops... Sorry about that. Fixed in the last diff.


https://reviews.llvm.org/D22946



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

Re: [PATCH] D23241: Add the notion of deferred diagnostics.

[Justin Lebar via cfe-commits]

jlebar updated this revision to Diff 67367.
jlebar marked an inline comment as done.
jlebar added a comment.

Address review comments, and don't abort codegen'ing a function if it has only
deferred warnings -- check specifically for errors.


https://reviews.llvm.org/D23241

Files:
  clang/include/clang/AST/Decl.h
  clang/lib/AST/Decl.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h

Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -490,6 +490,10 @@
   /// MDNodes.
   llvm::DenseMap MetadataIdMap;
 
+  /// Diags gathered from FunctionDecl::takeDeferredDiags().  Emitted at the
+  /// very end of codegen.
+  std::vector> DeferredDiags;
+
 public:
   CodeGenModule(ASTContext &C, const HeaderSearchOptions &headersearchopts,
 const PreprocessorOptions &ppopts,
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -497,6 +497,16 @@
   EmitVersionIdentMetadata();
 
   EmitTargetMetadata();
+
+  // Emit any deferred diagnostics gathered during codegen.  We didn't emit them
+  // when we first discovered them because that would have halted codegen,
+  // preventing us from gathering other deferred diags.
+  for (const PartialDiagnosticAt &DiagAt : DeferredDiags) {
+SourceLocation Loc = DiagAt.first;
+const PartialDiagnostic &PD = DiagAt.second;
+DiagnosticBuilder Builder(getDiags().Report(Loc, PD.getDiagID()));
+PD.Emit(Builder);
+  }
 }
 
 void CodeGenModule::UpdateCompletedType(const TagDecl *TD) {
@@ -2872,6 +2882,33 @@
  llvm::GlobalValue *GV) {
   const auto *D = cast(GD.getDecl());
 
+  // Emit this function's deferred diagnostics, if none of them are errors.  If
+  // any of them are errors, don't codegen the function, but also don't emit any
+  // of the diagnostics just yet.  Emitting an error during codegen stops
+  // further codegen, and we want to display as many deferred diags as possible.
+  // We'll emit the now twice-deferred diags at the very end of codegen.
+  //
+  // (If a function has both error and non-error diags, we don't emit the
+  // non-error diags here, because order can be significant, e.g. with notes
+  // that follow errors.)
+  auto Diags = D->takeDeferredDiags();
+  bool HasError = llvm::any_of(Diags, [this](const PartialDiagnosticAt &PDAt) {
+return getDiags().getDiagnosticLevel(PDAt.second.getDiagID(), PDAt.first) >=
+   DiagnosticsEngine::Error;
+  });
+  if (HasError) {
+DeferredDiags.insert(DeferredDiags.end(),
+ std::make_move_iterator(Diags.begin()),
+ std::make_move_iterator(Diags.end()));
+return;
+  }
+  for (PartialDiagnosticAt &PDAt : Diags) {
+const SourceLocation &Loc = PDAt.first;
+const PartialDiagnostic &PD = PDAt.second;
+DiagnosticBuilder Builder(getDiags().Report(Loc, PD.getDiagID()));
+PD.Emit(Builder);
+  }
+
   // Compute the function info and LLVM type.
   const CGFunctionInfo &FI = getTypes().arrangeGlobalDeclaration(GD);
   llvm::FunctionType *Ty = getTypes().GetFunctionType(FI);
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -3436,6 +3436,22 @@
   return 0;
 }
 
+void FunctionDecl::addDeferredDiag(PartialDiagnosticAt PD) {
+  if (!DeferredDiags)
+DeferredDiags = llvm::make_unique>();
+  DeferredDiags->emplace_back(PD);
+}
+
+std::vector FunctionDecl::takeDeferredDiags() const {
+  if (!DeferredDiags)
+return {};
+  assert(!DeferredDiags->empty() &&
+ "DeferredDiags should be non-null only if it's also non-empty.");
+  auto Ret = std::move(*DeferredDiags);
+  DeferredDiags.reset();
+  return Ret;
+}
+
 //===--===//
 // FieldDecl Implementation
 //===--===//
Index: clang/include/clang/AST/Decl.h
===
--- clang/include/clang/AST/Decl.h
+++ clang/include/clang/AST/Decl.h
@@ -1638,6 +1638,14 @@
   /// declaration name embedded in the DeclaratorDecl base class.
   DeclarationNameLoc DNLoc;
 
+  /// Storage for diagnostics deferred until this function is codegen'ed (if it
+  /// ever is).
+  ///
+  /// These are rarely used, so we use a pointer-to-vector to save two words
+  /// inside FunctionDecl.  This is mutable because emitting diagnostics (which
+  /// clears this list) needs to be a logically-const operation.
+  mutable std::unique_ptr> DeferredDiags;
+
   /// \brief Specify that this function declaration is actually a function
   /// template specialization.
   ///
@@ -2271

Re: [PATCH] D23241: Add the notion of deferred diagnostics.

[Justin Lebar via cfe-commits]

jlebar added a comment.

Reid, I'd still like you to have a look at this one if you don't mind, since 
it's outside my and Art's core competencies.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2886
@@ +2885,3 @@
+  // Check if this function has diagnostics that should be emitted when we
+  // codegen it.  If so, don't eit this function definition, but don't emit the
+  // diags just yet.  Emitting an error during codegen stops codegen, and we

tra wrote:
> eit->emit.  
> "don't do X, but don't do Y" construction sounds awkward to me.
> I'd reword the whole comment in terms of what the code does -- if there are 
> diagnostics, only collect them to emit at the end of codegen. Otherwise, 
> proceed to emit function definition.
> 
Thanks.  I tried to reword it, phal.

I also realized that this was skipping functions if they contained *any* 
deferred diags, but that's not right -- we only want to skip functions that 
contain deferred errors.  Fixed that too.

I wish I could use StoredDiagnostic instead of PartialDiagnosticAt, but I don't 
see a way to create a StoredDiagnostic for an error without setting the 
HasError bit in the DiagnosticEngine.  I need to carefully avoid setting that 
bit, otherwise we don't even make it to codegen.  (In fact it looks like the 
code for emitting diagnostics assumes that creating a StoredDiagnostic sets the 
bit, because I don't see it setting that bit when we emit the StoredDiagnostic.)


https://reviews.llvm.org/D23241



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

Re: [PATCH] D23279: clang-reorder-fields

[Kirill Bobyrev via cfe-commits]

omtcyfz added a comment.

In https://reviews.llvm.org/D23279#510061, @alexshap wrote:

> First of all, many thanks for the comments & proposal of clang-refactor.
>
> 1. Regarding high-level project organization: clang-refactor - that can be a 
> good place for various refactoring techniques like the existing clang-rename, 
> my simple tool, etc - essentially what Kirill said in the e-mail. I would be 
> happy to join. At the same time - if i can make some progress before this 
> reorganization is introduced - that would be helpful, if not - no problem at 
> all.
> 2. Regarding clang-tidy - it seems to me, that putting this particular tool 
> into PaddingChecker/clang-tidy is far from being perfect: A. In some cases 
> the optimal fields order is not unique, it would be more flexible to let the 
> user choose which order of fields he prefers. B. In some cases someone might 
> want to change the order of fields even if the padding is not affected at 
> all. C. Other concerns mentioned above in your comments


I would love to have a check, which reorders stuff while optimising it! Though, 
clang-tidy is limited to a single TU, too, which is not good.

Cheers! I now have a relevant example of what is refactoring and what is 
"addressing issues". So,

1. Optimising struct size is clearly "addressing issue". That's a good thing to 
have.
2. Reordering fields in struct is clearly refactoring. That's also a good thing 
to have.

First thing is a **issue addressing** task. I would love to have such tool to 
run over my whole codebase automatically from times to times.

As for the second tool - it performes a refactoring action by a user request, 
it has nothing common with **addressing issues**, which is what `clang-tidy` is 
meant for.

> One more thing: 

>  i am not sure i understood the comment  

>  "It actually breaks code ... only works while it is in the same TU".

>  For instance if we have the following project:

>  /include/Point.h (contains the definition of the struct Point)

>  /a.cpp (uses Point)

>  /b.cpp (also uses Point)

>  /main.cpp (also uses Point)

>  clang-reorder-fields -i -record-name ::Point -fields-order y,x main.cpp 
> a.cpp b.cpp

>  works for me.


Because everything is within a single Translation Unit :) Translation Unit is 
not equivalent to a file.

> At the same time i see in my code the following issue (easy to fix):

>  it will complain here :

> 

>   if (auto Err = Replacements[R.getFilePath()].add(R))

>  llvm::errs() << "Failed to add replacement, file: " << R.getFilePath()

> 

> because the same replacement for the code in header is being added multiple 
> times, 

>  but actually it doesn't affect the final set of changes and the result is 
> correct (i think we need either to handle this case more carefully or 
> (temporarily) add FIXME and replace llvm::errs with llvm::warns ).



Repository:
  rL LLVM

https://reviews.llvm.org/D23279



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

Re: [PATCH] D22946: [CUDA] Regression test to make sure C++ include path are forwarded to host and device frontends.

[Artem Belevich via cfe-commits]

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

LGTM.


https://reviews.llvm.org/D22946



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