[PATCH] D59798: [analyzer] Add analyzer option to limit the number of imported TUs

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

xazax.hun added a comment.

Mostly looks good, I have two slightly related nits.




Comment at: include/clang/CrossTU/CrossTranslationUnit.h:127
+   StringRef IndexName, bool DisplayCTUProgress,
+   unsigned CTULoadThreshold);
 

I would prefer to keep the default argument for `DisplayCTUProgress`. You could 
do this either by swapping the arguments or adding a default argument to 
`CTULoadThreshold` as well. I think I might prefer the latter. 



Comment at: include/clang/CrossTU/CrossTranslationUnit.h:176
   std::unique_ptr LookupTable;
+  unsigned NumASTLoaded{0u};
 };

Does it make sense to call `getCrossTUDefinition` with different thresholds for 
the same CTUContext? I suspect it is not, so it might make more sense to set 
the threshold in the context's constructor.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59798



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

[PATCH] D61822: make -ftime-trace also print template arguments

[Luboš Luňák via Phabricator via cfe-commits]

llunak created this revision.
llunak added a reviewer: anton-afanasyev.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Without this, I get e.g. 'PerformPendingInstantiations' -> 'std::fill', now I 
get 'std::fill'.


Repository:
  rC Clang

https://reviews.llvm.org/D61822

Files:
  lib/CodeGen/CodeGenModule.cpp
  lib/Parse/ParseDeclCXX.cpp
  lib/Sema/SemaTemplateInstantiate.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp


Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4156,7 +4156,11 @@
   }
 
   llvm::TimeTraceScope TimeScope("InstantiateFunction", [&]() {
-return Function->getQualifiedNameAsString();
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+Function->getNameForDiagnostic(OS, getPrintingPolicy(),
+   /*Qualified=*/true);
+return Name;
   });
 
   // If we're performing recursive template instantiation, create our own
Index: lib/Sema/SemaTemplateInstantiate.cpp
===
--- lib/Sema/SemaTemplateInstantiate.cpp
+++ lib/Sema/SemaTemplateInstantiate.cpp
@@ -2014,7 +2014,11 @@
 return true;
 
   llvm::TimeTraceScope TimeScope("InstantiateClass", [&]() {
-return Instantiation->getQualifiedNameAsString();
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+Instantiation->getNameForDiagnostic(OS, getPrintingPolicy(),
+/*Qualified=*/true);
+return Name;
   });
 
   Pattern = PatternDef;
Index: lib/Parse/ParseDeclCXX.cpp
===
--- lib/Parse/ParseDeclCXX.cpp
+++ lib/Parse/ParseDeclCXX.cpp
@@ -3154,8 +3154,13 @@
  TagType == DeclSpec::TST_class) && "Invalid TagType!");
 
   llvm::TimeTraceScope TimeScope("ParseClass", [&]() {
-if (auto *TD = dyn_cast_or_null(TagDecl))
-  return TD->getQualifiedNameAsString();
+if (auto *TD = dyn_cast_or_null(TagDecl)) {
+  std::string Name;
+  llvm::raw_string_ostream OS(Name);
+  TD->getNameForDiagnostic(OS, Actions.getASTContext().getPrintingPolicy(),
+   /*Qualified=*/true);
+  return Name;
+}
 return std::string("");
   });
 
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -2489,8 +2489,13 @@
 if (!shouldEmitFunction(GD))
   return;
 
-llvm::TimeTraceScope TimeScope(
-"CodeGen Function", [&]() { return FD->getQualifiedNameAsString(); });
+llvm::TimeTraceScope TimeScope("CodeGen Function", [&]() {
+  std::string Name;
+  llvm::raw_string_ostream OS(Name);
+  FD->getNameForDiagnostic(OS, getContext().getPrintingPolicy(),
+   /*Qualified=*/true);
+  return Name;
+});
 
 if (const auto *Method = dyn_cast(D)) {
   // Make sure to emit the definition(s) before we emit the thunks.


Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4156,7 +4156,11 @@
   }
 
   llvm::TimeTraceScope TimeScope("InstantiateFunction", [&]() {
-return Function->getQualifiedNameAsString();
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+Function->getNameForDiagnostic(OS, getPrintingPolicy(),
+   /*Qualified=*/true);
+return Name;
   });
 
   // If we're performing recursive template instantiation, create our own
Index: lib/Sema/SemaTemplateInstantiate.cpp
===
--- lib/Sema/SemaTemplateInstantiate.cpp
+++ lib/Sema/SemaTemplateInstantiate.cpp
@@ -2014,7 +2014,11 @@
 return true;
 
   llvm::TimeTraceScope TimeScope("InstantiateClass", [&]() {
-return Instantiation->getQualifiedNameAsString();
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+Instantiation->getNameForDiagnostic(OS, getPrintingPolicy(),
+/*Qualified=*/true);
+return Name;
   });
 
   Pattern = PatternDef;
Index: lib/Parse/ParseDeclCXX.cpp
===
--- lib/Parse/ParseDeclCXX.cpp
+++ lib/Parse/ParseDeclCXX.cpp
@@ -3154,8 +3154,13 @@
  TagType == DeclSpec::TST_class) && "Invalid TagType!");
 
   llvm::TimeTraceScope TimeScope("ParseClass", [&]() {
-if (auto *TD = dyn_cast_or_null(TagDecl))
-  return TD->getQualifiedNameAsString();
+if (auto *TD = dyn_cast_or_null(TagDecl)) {
+  std::string Name;
+  llvm::raw_string_ostream OS(Name);
+  TD->getNameForDiagnostic(OS, Actions.getASTContext().getPrintingPolicy(

[PATCH] D61749: [clang-tidy] initial version of readability-static-const-method

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a subscriber: shuaiwang.
JonasToth added a comment.

Hey, I do like your check!

I think it would be great to implement this analysis in `utils/` as roman 
suggested. Then we have bigger flexibility.
The `ExprMutAnalyzer` was requested to be moved to `clang/Analysis/` as the CSA 
folks wanted to reuse it at some point. That might be the case with your logic 
as well, as it is still related to `ExprMutAnalyzer` even though its not the 
same.

Another issue why the clang-tidy part for the variable const-suggestions 
doesn't move forward is that `ExprMutAnalyzer` itself is blocked due to 
@shuaiwang not having time as well. We have a view nasty cases that produce 
false positives and especially pointer analysis is complicated.

For that I think that @mgehre approach of producing something useful now should 
not be blocked on us. Especially since its unclear when we can continue our 
work substantially. Preparing for joining forces later would be great though! I 
think this is possible with having just a class/set of functions for the 
analysis that can be used in different parts of clang tools is sufficient for 
that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61749



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

[PATCH] D61749: [clang-tidy] initial version of readability-static-const-method

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/readability-static-const-method.cpp:209
+void KeepLambdas() {
+  auto F = +[]() { return 0; };
+  auto F2 = []() { return 0; };

Does it pass here?
I looks a bit weird, shouldnt the lambda be called for this to work (this is 
the unary + right?)?



Comment at: 
clang-tools-extra/test/clang-tidy/readability-static-const-method.cpp:312
+return const_cast(this)->get();
+  }
+

I think more template tests wouldn't hurt. From the other checks experience I 
can safely say we got burned a few times :)
Instantiating the templates with builtin arrays, pointers, references and 
different qualifiers usually produces interesting test cases as well, that need 
to be handled properly.

Another thing that comes to mind with templates are overloaded operators.
```
template 
void bar() {
 Foo x1, x2;
 Foo y = x1 + x2;
}
```
Builtins are not changed by `operator+` but that can not be said about other 
types in general (maybe with concepts used properly).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61749



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

r360515 - ConstantLValueEmitter::tryEmitAbsolute - remove unused variable. NFCI.

[Simon Pilgrim via cfe-commits]

Author: rksimon
Date: Sat May 11 04:01:46 2019
New Revision: 360515

URL: http://llvm.org/viewvc/llvm-project?rev=360515&view=rev
Log:
ConstantLValueEmitter::tryEmitAbsolute - remove unused variable. NFCI.

Duplicate getOffset() call.

Modified:
cfe/trunk/lib/CodeGen/CGExprConstant.cpp

Modified: cfe/trunk/lib/CodeGen/CGExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprConstant.cpp?rev=360515&r1=360514&r2=360515&view=diff
==
--- cfe/trunk/lib/CodeGen/CGExprConstant.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprConstant.cpp Sat May 11 04:01:46 2019
@@ -1695,8 +1695,6 @@ llvm::Constant *ConstantLValueEmitter::t
 /// bitcast to pointer type.
 llvm::Constant *
 ConstantLValueEmitter::tryEmitAbsolute(llvm::Type *destTy) {
-  auto offset = getOffset();
-
   // If we're producing a pointer, this is easy.
   auto destPtrTy = cast(destTy);
   if (Value.isNullPointer()) {
@@ -1708,7 +1706,7 @@ ConstantLValueEmitter::tryEmitAbsolute(l
   // to a pointer.
   // FIXME: signedness depends on the original integer type.
   auto intptrTy = CGM.getDataLayout().getIntPtrType(destPtrTy);
-  llvm::Constant *C = offset;
+  llvm::Constant *C;
   C = llvm::ConstantExpr::getIntegerCast(getOffset(), intptrTy,
  /*isSigned*/ false);
   C = llvm::ConstantExpr::getIntToPtr(C, destPtrTy);


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

[PATCH] D61822: make -ftime-trace also print template arguments

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 199133.
llunak added a comment.

Removed the parse case, getNameForDiagnostic() apparently prints nothing there.


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

https://reviews.llvm.org/D61822

Files:
  lib/CodeGen/CodeGenModule.cpp
  lib/Sema/SemaTemplateInstantiate.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp


Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4156,7 +4156,11 @@
   }
 
   llvm::TimeTraceScope TimeScope("InstantiateFunction", [&]() {
-return Function->getQualifiedNameAsString();
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+Function->getNameForDiagnostic(OS, getPrintingPolicy(),
+   /*Qualified=*/true);
+return Name;
   });
 
   // If we're performing recursive template instantiation, create our own
Index: lib/Sema/SemaTemplateInstantiate.cpp
===
--- lib/Sema/SemaTemplateInstantiate.cpp
+++ lib/Sema/SemaTemplateInstantiate.cpp
@@ -2014,7 +2014,11 @@
 return true;
 
   llvm::TimeTraceScope TimeScope("InstantiateClass", [&]() {
-return Instantiation->getQualifiedNameAsString();
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+Instantiation->getNameForDiagnostic(OS, getPrintingPolicy(),
+/*Qualified=*/true);
+return Name;
   });
 
   Pattern = PatternDef;
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -2489,8 +2489,13 @@
 if (!shouldEmitFunction(GD))
   return;
 
-llvm::TimeTraceScope TimeScope(
-"CodeGen Function", [&]() { return FD->getQualifiedNameAsString(); });
+llvm::TimeTraceScope TimeScope("CodeGen Function", [&]() {
+  std::string Name;
+  llvm::raw_string_ostream OS(Name);
+  FD->getNameForDiagnostic(OS, getContext().getPrintingPolicy(),
+   /*Qualified=*/true);
+  return Name;
+});
 
 if (const auto *Method = dyn_cast(D)) {
   // Make sure to emit the definition(s) before we emit the thunks.


Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4156,7 +4156,11 @@
   }
 
   llvm::TimeTraceScope TimeScope("InstantiateFunction", [&]() {
-return Function->getQualifiedNameAsString();
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+Function->getNameForDiagnostic(OS, getPrintingPolicy(),
+   /*Qualified=*/true);
+return Name;
   });
 
   // If we're performing recursive template instantiation, create our own
Index: lib/Sema/SemaTemplateInstantiate.cpp
===
--- lib/Sema/SemaTemplateInstantiate.cpp
+++ lib/Sema/SemaTemplateInstantiate.cpp
@@ -2014,7 +2014,11 @@
 return true;
 
   llvm::TimeTraceScope TimeScope("InstantiateClass", [&]() {
-return Instantiation->getQualifiedNameAsString();
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+Instantiation->getNameForDiagnostic(OS, getPrintingPolicy(),
+/*Qualified=*/true);
+return Name;
   });
 
   Pattern = PatternDef;
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -2489,8 +2489,13 @@
 if (!shouldEmitFunction(GD))
   return;
 
-llvm::TimeTraceScope TimeScope(
-"CodeGen Function", [&]() { return FD->getQualifiedNameAsString(); });
+llvm::TimeTraceScope TimeScope("CodeGen Function", [&]() {
+  std::string Name;
+  llvm::raw_string_ostream OS(Name);
+  FD->getNameForDiagnostic(OS, getContext().getPrintingPolicy(),
+   /*Qualified=*/true);
+  return Name;
+});
 
 if (const auto *Method = dyn_cast(D)) {
   // Make sure to emit the definition(s) before we emit the thunks.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53072: [clang-format] Create a new tool for IDEs based on clang-format

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan added a comment.

@sammccall 
I can't avoid adding extra formatting options because my first attempt to 
introduce an ordinary clang-format option faced resistance because of not 
fitting the clang-format purpose to format files.


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

https://reviews.llvm.org/D53072



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

[PATCH] D61822: make -ftime-trace also print template arguments

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev accepted this revision.
anton-afanasyev added a comment.
This revision is now accepted and ready to land.

LGTM
Thanks!


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

https://reviews.llvm.org/D61822



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

[PATCH] D61786: [ASTImporter] Separate unittest files

[Aleksei Sidorin via Phabricator via cfe-commits]

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

Hi Gabor,
I like this change! LGTM, just a few nits.




Comment at: clang/unittests/AST/ASTImporterGenericRedeclTest.cpp:20
+
+// FIXME put these structs and the tests rely on them into their own separate
+// test file!

Is this FIXME obsolete now?



Comment at: clang/unittests/AST/ASTImporterVisibilityTest.cpp:37
+// FunctionDecl:
+auto *ExternF = "void f();";
+auto *StaticF = "static void f();";

Can we use StringRef (or, at least `const auto *`)?



Comment at: clang/unittests/AST/ASTImporterVisibilityTest.cpp:49
+::testing::WithParamInterface>;
+// Fixture to test the redecl chain of Decls with the same visibility.  Gtest
+// makes it possible to have either value-parameterized or type-parameterized

Double spaces in the end of sentences look a bit strange.



Comment at: clang/unittests/AST/ASTImporterVisibilityTest.cpp:139
+
+auto *ToF0 = FirstDeclMatcher().match(ToTu, getPattern());
+auto *FromF1 = FirstDeclMatcher().match(FromTu, getPattern());

Optional: F0/F1 (where F stands for "function", I guess) can be replaced with 
D0/D1 (for decl) since the code is generic.



Comment at: clang/unittests/AST/ASTImporterVisibilityTest.cpp:157
+TranslationUnitDecl *FromTu1 = getTuDecl(getCode1(), Lang_CXX, 
"input1.cc");
+auto *FromF0 =
+FirstDeclMatcher().match(FromTu0, getPattern());

This should fit a single line.



Comment at: clang/unittests/AST/ASTImporterVisibilityTest.cpp:190
+
+bool ExpectLink = true;
+bool ExpectNotLink = false;

const?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61786



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

[PATCH] D44100: [ASTImporter] Reorder fields after structure import is finished

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin added a comment.

Thank you for digging into this! Feel free to ask me if you encounter any 
problems with the patch.


Repository:
  rC Clang

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

https://reviews.llvm.org/D44100



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

[PATCH] D61815: [CFG] NFC: Modernize test/Analysis/initializers-cfg-output.cpp.

[Aleksei Sidorin via Phabricator via cfe-commits]

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

Wow, this is a cool idea!


Repository:
  rC Clang

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

https://reviews.llvm.org/D61815



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

[PATCH] D61814: [CFG] NFC: Remove implicit conversion from CFGTerminator to Stmt *, make it a variant class instead.

[Aleksei Sidorin via Phabricator via cfe-commits]

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

The conversion operator indeed looks non-evident.




Comment at: clang/include/clang/Analysis/CFG.h:513
 public:
-  CFGTerminator() = default;
-  CFGTerminator(Stmt *S, bool TemporaryDtorsBranch = false)
-  : Data(S, TemporaryDtorsBranch) {}
+  CFGTerminator() { assert(!isValid()); }
+  CFGTerminator(Stmt *S, Kind K = StmtBranch) : Data(S, K) {}

It seems to me that `isStmt()` and `isTemporaryDtorBranch()` methods can make 
the code a bit cleaner in few places by avoiding comparisons with enums.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61814



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

[PATCH] D53072: [clang-format] Create a new tool for IDEs based on clang-format

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan updated this revision to Diff 199150.
yvvan added a comment.

Sorry for unrelated formatting changes - that's clang-format's work :)

I've removed the extra executable.

I don't know how to force that behavior only for the given line (for that I 
need someone who can help) but in our usecase we should not care if we apply 
the rule to the whole file or only to the current line since we are only 
interested in one single Replacement which we can anyways filter afterwards.


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

https://reviews.llvm.org/D53072

Files:
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatInternal.h
  clang/lib/Format/TokenAnalyzer.cpp
  clang/lib/Format/TokenAnalyzer.h
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineFormatter.h
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/tools/clang-format/ClangFormat.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -29,21 +29,18 @@
 
 class FormatTest : public ::testing::Test {
 protected:
-  enum StatusCheck {
-SC_ExpectComplete,
-SC_ExpectIncomplete,
-SC_DoNotCheck
-  };
+  enum StatusCheck { SC_ExpectComplete, SC_ExpectIncomplete, SC_DoNotCheck };
 
-  std::string format(llvm::StringRef Code,
- const FormatStyle &Style = getLLVMStyle(),
- StatusCheck CheckComplete = SC_ExpectComplete) {
+  std::string
+  format(llvm::StringRef Code, const FormatStyle &Style = getLLVMStyle(),
+ StatusCheck CheckComplete = SC_ExpectComplete,
+ ExtraFormattingOptions ExtraOptions = ExtraFormattingOptions::None) {
 LLVM_DEBUG(llvm::errs() << "---\n");
 LLVM_DEBUG(llvm::errs() << Code << "\n\n");
 std::vector Ranges(1, tooling::Range(0, Code.size()));
 FormattingAttemptStatus Status;
 tooling::Replacements Replaces =
-reformat(Style, Code, Ranges, "", &Status);
+reformat(Style, ExtraOptions, Code, Ranges, "", &Status);
 if (CheckComplete != SC_DoNotCheck) {
   bool ExpectedCompleteFormat = CheckComplete == SC_ExpectComplete;
   EXPECT_EQ(ExpectedCompleteFormat, Status.FormatComplete)
@@ -332,13 +329,15 @@
 format("namespace {\n"
"int i;\n"
"\n"
-   "}", LLVMWithNoNamespaceFix));
+   "}",
+   LLVMWithNoNamespaceFix));
   EXPECT_EQ("namespace {\n"
 "int i;\n"
 "}",
 format("namespace {\n"
"int i;\n"
-   "}", LLVMWithNoNamespaceFix));
+   "}",
+   LLVMWithNoNamespaceFix));
   EXPECT_EQ("namespace {\n"
 "int i;\n"
 "\n"
@@ -346,13 +345,15 @@
 format("namespace {\n"
"int i;\n"
"\n"
-   "};", LLVMWithNoNamespaceFix));
+   "};",
+   LLVMWithNoNamespaceFix));
   EXPECT_EQ("namespace {\n"
 "int i;\n"
 "};",
 format("namespace {\n"
"int i;\n"
-   "};", LLVMWithNoNamespaceFix));
+   "};",
+   LLVMWithNoNamespaceFix));
   EXPECT_EQ("namespace {\n"
 "int i;\n"
 "\n"
@@ -395,6 +396,25 @@
Style));
 }
 
+TEST_F(FormatTest, KeepsLineBreaks) {
+  FormatStyle Style = getLLVMStyle();
+  EXPECT_EQ("if (a\n"
+"&& b) {\n"
+"}",
+format("if (a\n"
+   "&& b) {\n"
+   "}",
+   Style, SC_ExpectComplete,
+   ExtraFormattingOptions::KeepLineBreaksForNonEmptyLines));
+
+  EXPECT_EQ("[]() {\n"
+"  foo(); }",
+format("[]() {\n"
+   "foo(); }",
+   Style, SC_ExpectComplete,
+   ExtraFormattingOptions::KeepLineBreaksForNonEmptyLines));
+}
+
 TEST_F(FormatTest, RecognizesBinaryOperatorKeywords) {
   verifyFormat("x = (a) and (b);");
   verifyFormat("x = (a) or (b);");
@@ -854,7 +874,8 @@
   verifyFormat("for (Foo *x = 0; x != in; x++) {\n}");
   verifyFormat("Foo *x;\nfor (x = 0; x != in; x++) {\n}");
   verifyFormat("Foo *x;\nfor (x in y) {\n}");
-  verifyFormat("for (const Foo &baz = in.value(); !baz.at_end(); ++baz) {\n}");
+  verifyFormat(
+  "for (const Foo &baz = in.value(); !baz.at_end(); ++baz) {\n}");
 
   FormatStyle NoBinPacking = getLLVMStyle();
   NoBinPacking.BinPackParameters = false;
@@ -1246,20 +1267,26 @@
"#endif\n"
"}",
Style));
-  EXPECT_EQ("switch (a) {\n" "case 0:\n"
-"  re

[PATCH] D53072: [clang-format] Create a new tool for IDEs based on clang-format

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan updated this revision to Diff 199151.
yvvan added a comment.

Some misleading reformatting fixed.


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

https://reviews.llvm.org/D53072

Files:
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatInternal.h
  clang/lib/Format/TokenAnalyzer.cpp
  clang/lib/Format/TokenAnalyzer.h
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineFormatter.h
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/tools/clang-format/ClangFormat.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -35,15 +35,16 @@
 SC_DoNotCheck
   };
 
-  std::string format(llvm::StringRef Code,
- const FormatStyle &Style = getLLVMStyle(),
- StatusCheck CheckComplete = SC_ExpectComplete) {
+  std::string
+  format(llvm::StringRef Code, const FormatStyle &Style = getLLVMStyle(),
+ StatusCheck CheckComplete = SC_ExpectComplete,
+ ExtraFormattingOptions ExtraOptions = ExtraFormattingOptions::None) {
 LLVM_DEBUG(llvm::errs() << "---\n");
 LLVM_DEBUG(llvm::errs() << Code << "\n\n");
 std::vector Ranges(1, tooling::Range(0, Code.size()));
 FormattingAttemptStatus Status;
 tooling::Replacements Replaces =
-reformat(Style, Code, Ranges, "", &Status);
+reformat(Style, ExtraOptions, Code, Ranges, "", &Status);
 if (CheckComplete != SC_DoNotCheck) {
   bool ExpectedCompleteFormat = CheckComplete == SC_ExpectComplete;
   EXPECT_EQ(ExpectedCompleteFormat, Status.FormatComplete)
@@ -395,6 +396,25 @@
Style));
 }
 
+TEST_F(FormatTest, KeepsLineBreaks) {
+  FormatStyle Style = getLLVMStyle();
+  EXPECT_EQ("if (a\n"
+"&& b) {\n"
+"}",
+format("if (a\n"
+   "&& b) {\n"
+   "}",
+   Style, SC_ExpectComplete,
+   ExtraFormattingOptions::KeepLineBreaksForNonEmptyLines));
+
+  EXPECT_EQ("[]() {\n"
+"  foo(); }",
+format("[]() {\n"
+   "foo(); }",
+   Style, SC_ExpectComplete,
+   ExtraFormattingOptions::KeepLineBreaksForNonEmptyLines));
+}
+
 TEST_F(FormatTest, RecognizesBinaryOperatorKeywords) {
   verifyFormat("x = (a) and (b);");
   verifyFormat("x = (a) or (b);");
Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -51,13 +51,14 @@
  "Can only be used with one input file."),
 cl::cat(ClangFormatCategory));
 static cl::list
-LineRanges("lines", cl::desc(": - format a range of\n"
- "lines (both 1-based).\n"
- "Multiple ranges can be formatted by specifying\n"
- "several -lines arguments.\n"
- "Can't be used with -offset and -length.\n"
- "Can only be used with one input file."),
-   cl::cat(ClangFormatCategory));
+LineRanges("lines",
+   cl::desc(": - format a range of\n"
+"lines (both 1-based).\n"
+"Multiple ranges can be formatted by specifying\n"
+"several -lines arguments.\n"
+"Can't be used with -offset and -length.\n"
+"Can only be used with one input file."),
+   cl::cat(ClangFormatCategory));
 static cl::opt
 Style("style", cl::desc(clang::format::StyleOptionHelpDescription),
   cl::init(clang::format::DefaultFormatStyle),
@@ -72,12 +73,12 @@
   cl::init(clang::format::DefaultFallbackStyle),
   cl::cat(ClangFormatCategory));
 
-static cl::opt
-AssumeFileName("assume-filename",
-   cl::desc("When reading from stdin, clang-format assumes this\n"
-"filename to look for a style config file (with\n"
-"-style=file) and to determine the language."),
-   cl::init(""), cl::cat(ClangFormatCategory));
+static cl::opt AssumeFileName(
+"assume-filename",
+cl::desc("When reading from stdin, clang-format assumes this\n"
+ "filename to look for a style config file (with\n"
+ "-style=file) and to determine the language."),
+cl::init(""), cl::cat(ClangFormatCategory));
 
 static cl::opt Inplace("i",
  cl::desc("Inplace edit s, if specified."),
@@ -107,6 +108,11 @@
 Verbose("verbose", cl::desc("If set, shows the lis

[PATCH] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

[Torbjörn Klatt via Phabricator via cfe-commits]

torbjoernk created this revision.
torbjoernk added reviewers: dergachev.a, hintonda, alexfh.
torbjoernk added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.

modernize-loop-convert was not detecting implicit casts to
const_iterator as convertible to range-based loops:

  std::vector vec{1,2,3,4}
  for(std::vector::const_iterator i = vec.begin();
  i != vec.end();
  ++i) { }

Thanks to Don Hinton for advice on cfe-dev.

Also simplify AST matcher for matching iterator-based loops.

Thanks to Artem Dergachev for spotting it on cfe-dev.

Fixes PR#35082


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D61827

Files:
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/test/clang-tidy/modernize-loop-convert-basic.cpp


Index: clang-tools-extra/test/clang-tidy/modernize-loop-convert-basic.cpp
===
--- clang-tools-extra/test/clang-tidy/modernize-loop-convert-basic.cpp
+++ clang-tools-extra/test/clang-tidy/modernize-loop-convert-basic.cpp
@@ -270,6 +270,13 @@
   // CHECK-FIXES: for (auto & P : *Ps)
   // CHECK-FIXES-NEXT: printf("s has value %d\n", P.X);
 
+  for (S::const_iterator It = Ss.begin(), E = Ss.end(); It != E; ++It) {
+printf("s has value %d\n", (*It).X);
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (auto It : Ss)
+  // CHECK-FIXES-NEXT: printf("s has value %d\n", It.X);
+
   for (S::const_iterator It = Ss.cbegin(), E = Ss.cend(); It != E; ++It) {
 printf("s has value %d\n", (*It).X);
   }
Index: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
@@ -128,10 +128,7 @@
   .bind(BeginCallName);
 
   DeclarationMatcher InitDeclMatcher =
-  varDecl(hasInitializer(anyOf(ignoringParenImpCasts(BeginCallMatcher),
-   materializeTemporaryExpr(
-   
ignoringParenImpCasts(BeginCallMatcher)),
-   hasDescendant(BeginCallMatcher
+  varDecl(hasInitializer(hasDescendant(BeginCallMatcher)))
   .bind(InitVarName);
 
   DeclarationMatcher EndDeclMatcher =
@@ -791,11 +788,6 @@
   CanonicalBeginType->getPointeeType(),
   CanonicalInitVarType->getPointeeType()))
 return false;
-} else if (!Context->hasSameType(CanonicalInitVarType,
- CanonicalBeginType)) {
-  // Check for qualified types to avoid conversions from non-const to const
-  // iterator types.
-  return false;
 }
   } else if (FixerKind == LFK_PseudoArray) {
 // This call is required to obtain the container.


Index: clang-tools-extra/test/clang-tidy/modernize-loop-convert-basic.cpp
===
--- clang-tools-extra/test/clang-tidy/modernize-loop-convert-basic.cpp
+++ clang-tools-extra/test/clang-tidy/modernize-loop-convert-basic.cpp
@@ -270,6 +270,13 @@
   // CHECK-FIXES: for (auto & P : *Ps)
   // CHECK-FIXES-NEXT: printf("s has value %d\n", P.X);
 
+  for (S::const_iterator It = Ss.begin(), E = Ss.end(); It != E; ++It) {
+printf("s has value %d\n", (*It).X);
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (auto It : Ss)
+  // CHECK-FIXES-NEXT: printf("s has value %d\n", It.X);
+
   for (S::const_iterator It = Ss.cbegin(), E = Ss.cend(); It != E; ++It) {
 printf("s has value %d\n", (*It).X);
   }
Index: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
@@ -128,10 +128,7 @@
   .bind(BeginCallName);
 
   DeclarationMatcher InitDeclMatcher =
-  varDecl(hasInitializer(anyOf(ignoringParenImpCasts(BeginCallMatcher),
-   materializeTemporaryExpr(
-   ignoringParenImpCasts(BeginCallMatcher)),
-   hasDescendant(BeginCallMatcher
+  varDecl(hasInitializer(hasDescendant(BeginCallMatcher)))
   .bind(InitVarName);
 
   DeclarationMatcher EndDeclMatcher =
@@ -791,11 +788,6 @@
   CanonicalBeginType->getPointeeType(),
   CanonicalInitVarType->getPointeeType()))
 return false;
-} else if (!Context->hasSameType(CanonicalInitVarType,
- CanonicalBeginType)) {
-  // Check for qualified types to avoid conversions from non-const to const
-  // iterator types.
-  return false;
 }
   } else i

[PATCH] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

[Don Hinton via Phabricator via cfe-commits]

hintonda added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/modernize-loop-convert-basic.cpp:273
 
+  for (S::const_iterator It = Ss.begin(), E = Ss.end(); It != E; ++It) {
+printf("s has value %d\n", (*It).X);

I think you need to fix the comment and add checks to these also, which is what 
the `if else` was dealing with:

   434// V.begin() returns a user-defined type 'iterator' which, since it's
   435// different from const_iterator, disqualifies these loops from
   436// transformation.
   437dependent V;
   438for (dependent::const_iterator It = V.begin(), E = V.end();
   439 It != E; ++It) {
   440  printf("Fibonacci number is %d\n", *It);
   441}
   442
   443for (dependent::const_iterator It(V.begin()), E = V.end();
   444 It != E; ++It) {
   445  printf("Fibonacci number is %d\n", *It);
   446}
   447  }



Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61827



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

[PATCH] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

This will now trigger on https://godbolt.org/z/9oFMcB right?
Just want to point out that this will then have "false-positives" when that loop
is an OpenMP for loop, since range-for loop is not available until OpenMP 5.

I don't think this false-positive can be avoided though, if building without
`-fopenmp` there won't be anything about OpenMP in AST,
and thus no way to detect this case..


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61827



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

[PATCH] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D61827#1499183 , @hintonda wrote:

> In D61827#1499160 , @lebedev.ri 
> wrote:
>
> > This will now trigger on https://godbolt.org/z/9oFMcB right?
> >  Just want to point out that this will then have "false-positives" when 
> > that loop
> >  is an OpenMP for loop, since range-for loop is not available until OpenMP 
> > 5.
> >
> > I don't think this false-positive can be avoided though, if building without
> >  `-fopenmp` there won't be anything about OpenMP in AST,
> >  and thus no way to detect this case..
>
>
> Could you suggest a simple test case that could be added to the test?  That 
> way, instead of just removing the `if else` block, @torbjoernk could try to 
> handle it.  Or perhaps exclude it from the match altogether.


As i said, i don't see how this can be avoided in general.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61827



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

[PATCH] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D61827#1499160 , @lebedev.ri wrote:

> This will now trigger on https://godbolt.org/z/9oFMcB right?
>  Just want to point out that this will then have "false-positives" when that 
> loop
>  is an OpenMP for loop, since range-for loop is not available until OpenMP 5.
>
> I don't think this false-positive can be avoided though, if building without
>  `-fopenmp` there won't be anything about OpenMP in AST,
>  and thus no way to detect this case..


Could you suggest a simple test case that could be added to the test?  That 
way, instead of just removing the `if else` block, @torbjoernk could try to 
handle it.  Or perhaps exclude it from the match altogether.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61827



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

r360531 - Revert rL360499 and rL360464 from cfe/trunk:

[Simon Pilgrim via cfe-commits]

Author: rksimon
Date: Sat May 11 13:21:59 2019
New Revision: 360531

URL: http://llvm.org/viewvc/llvm-project?rev=360531&view=rev
Log:
Revert rL360499 and rL360464 from cfe/trunk:
Reject attempts to call non-static member functions on objects outside
their lifetime in constant expressions.

This is undefined behavior per [class.cdtor]p2.

We continue to allow this for objects whose values are not visible
within the constant evaluation, because there's no way we can tell
whether the access is defined or not, existing code relies on the
ability to make such calls, and every other compiler allows such
calls.

Fix handling of objects under construction during constant expression
evaluation.

It's not enough to just track the LValueBase that we're evaluating, we
need to also track the path to the objects whose constructors are
running.

Fixes windows buildbots

Modified:
cfe/trunk/include/clang/AST/APValue.h
cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
cfe/trunk/lib/AST/APValue.cpp
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/test/CXX/expr/expr.const/p2-0x.cpp
cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp
cfe/trunk/test/SemaCXX/constant-expression-cxx1y.cpp

Modified: cfe/trunk/include/clang/AST/APValue.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/APValue.h?rev=360531&r1=360530&r2=360531&view=diff
==
--- cfe/trunk/include/clang/AST/APValue.h (original)
+++ cfe/trunk/include/clang/AST/APValue.h Sat May 11 13:21:59 2019
@@ -96,14 +96,10 @@ public:
   return Version;
 }
 
-friend bool operator==(const LValueBase &LHS, const LValueBase &RHS) {
-  return LHS.Ptr == RHS.Ptr && LHS.CallIndex == RHS.CallIndex &&
- LHS.Version == RHS.Version;
+bool operator==(const LValueBase &Other) const {
+  return Ptr == Other.Ptr && CallIndex == Other.CallIndex &&
+ Version == Other.Version;
 }
-friend bool operator!=(const LValueBase &LHS, const LValueBase &RHS) {
-  return !(LHS == RHS);
-}
-friend llvm::hash_code hash_value(const LValueBase &Base);
 
   private:
 PtrTy Ptr;

Modified: cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td?rev=360531&r1=360530&r2=360531&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td Sat May 11 13:21:59 2019
@@ -67,13 +67,13 @@ def note_constexpr_past_end : Note<
   "%select{temporary|%2}1 is not a constant expression">;
 def note_constexpr_past_end_subobject : Note<
   "cannot %select{access base class of|access derived class of|access field 
of|"
-  "access array element of|ERROR|"
+  "access array element of|ERROR|call member function on|"
   "access real component of|access imaginary component of}0 "
   "pointer past the end of object">;
 def note_constexpr_null_subobject : Note<
   "cannot %select{access base class of|access derived class of|access field 
of|"
   "access array element of|perform pointer arithmetic on|"
-  "access real component of|"
+  "call member function on|access real component of|"
   "access imaginary component of}0 null pointer">;
 def note_constexpr_var_init_non_constant : Note<
   "initializer of %0 is not a constant expression">;
@@ -96,10 +96,10 @@ def note_constexpr_this : Note<
   "%select{|implicit }0use of 'this' pointer is only allowed within the "
   "evaluation of a call to a 'constexpr' member function">;
 def note_constexpr_lifetime_ended : Note<
-  "%select{read of|assignment to|increment of|decrement of|member call on}0 "
+  "%select{read of|assignment to|increment of|decrement of}0 "
   "%select{temporary|variable}1 whose lifetime has ended">;
 def note_constexpr_access_uninit : Note<
-  "%select{read of|assignment to|increment of|decrement of|member call on}0 "
+  "%select{read of|assignment to|increment of|decrement of}0 "
   "object outside its lifetime is not allowed in a constant expression">;
 def note_constexpr_use_uninit_reference : Note<
   "use of reference outside its lifetime "
@@ -108,14 +108,12 @@ def note_constexpr_modify_const_type : N
   "modification of object of const-qualified type %0 is not allowed "
   "in a constant expression">;
 def note_constexpr_access_volatile_type : Note<
-  "%select{read of|assignment to|increment of|decrement of|}0 "
+  "%select{read of|assignment to|increment of|decrement of}0 "
   "volatile-qualified type %1 is not allowed in a constant expression">;
 def note_constexpr_access_volatile_obj : Note<
-  "%select{read of|assignment to|increment of|decrement of|}0 volatile "
+  "%select{read of|assignment to|increment of|decrement of}0 volatile "
   "%select{temporary|object %2|member %2}1 is not allowed in "
   "a constant expre