[PATCH] D45680: [C++2a] Add operator<=> Rewriting - Early Attempt

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added inline comments.



Comment at: lib/Sema/SemaOverload.cpp:9177
+// specified in C++2a [over.match.oper]p1.10.
+if (RewrittenCandidateTieBreaker && ICS1.isStandard() &&
+ICS2.isStandard() && ICS1.Standard.getRank() == ICR_Exact_Match &&

This doesn't seem anywhere close to correct, but It's the best I've come up 
with so far.
For now though, it seems nice to avoid the ambiguity caused by the current 
specification.


https://reviews.llvm.org/D45680



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

[PATCH] D45680: [C++2a] Add operator<=> Rewriting - Early Attempt

[Eric Fiselier via Phabricator via cfe-commits]

EricWF updated this revision to Diff 145390.
EricWF marked 7 inline comments as done.
EricWF added a comment.

- Remove the `= default` changes as requested.

- Attempt to work around ambiguity in overload resolution caused by differing 
lvalue or qualification conversions when ranking synthesized candidates. For 
example:

  struct T {};
  auto operator<=>(T const&, T&&);
  T{} <=> T{}; // Ambiguous according to the current spec.

The workaround ignores worse implicit conversions when the conversions are both 
an exact match and a rewritten candidate tie-breaker is present.


https://reviews.llvm.org/D45680

Files:
  include/clang/AST/ExprCXX.h
  include/clang/AST/RecursiveASTVisitor.h
  include/clang/Basic/StmtNodes.td
  include/clang/Sema/Overload.h
  include/clang/Sema/Sema.h
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/Expr.cpp
  lib/AST/ExprClassification.cpp
  lib/AST/ExprConstant.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/StmtPrinter.cpp
  lib/AST/StmtProfile.cpp
  lib/CodeGen/CGExprScalar.cpp
  lib/Sema/SemaExceptionSpec.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/CodeGenCXX/cxx2a-compare.cpp
  test/SemaCXX/compare-cxx2a.cpp

Index: test/SemaCXX/compare-cxx2a.cpp
===
--- test/SemaCXX/compare-cxx2a.cpp
+++ test/SemaCXX/compare-cxx2a.cpp
@@ -293,20 +293,21 @@
 
 template 
 struct Tag {};
-// expected-note@+1 {{candidate}}
-Tag<0> operator<=>(EnumA, EnumA) {
-  return {};
+std::strong_ordering operator<=>(EnumA, EnumA) {
+  return std::strong_ordering::equal;
 }
-Tag<1> operator<=>(EnumA, EnumB) {
-  return {};
+// expected-note@+1 {{candidate function}},
+std::strong_ordering operator<=>(EnumA a, EnumB b) {
+  return ((int)a <=> (int)b);
 }
 
 void test_enum_ovl_provided() {
   auto r1 = (EnumA::A <=> EnumA::A);
-  ASSERT_EXPR_TYPE(r1, Tag<0>);
+  ASSERT_EXPR_TYPE(r1, std::strong_ordering);
   auto r2 = (EnumA::A <=> EnumB::B);
-  ASSERT_EXPR_TYPE(r2, Tag<1>);
-  (void)(EnumB::B <=> EnumA::A); // expected-error {{invalid operands to binary expression ('EnumCompareTests::EnumB' and 'EnumCompareTests::EnumA')}}
+  ASSERT_EXPR_TYPE(r2, std::strong_ordering);
+  (void)(EnumB::B <=> EnumA::A); // OK, chooses reverse order synthesized candidate.
+  (void)(EnumB::B <=> EnumC::C); // expected-error {{invalid operands to binary expression ('EnumCompareTests::EnumB' and 'EnumCompareTests::EnumC')}}
 }
 
 void enum_float_test() {
@@ -422,3 +423,125 @@
 }
 
 } // namespace ComplexTest
+
+namespace TestRewritting {
+
+struct T {
+  int x;
+  // expected-note@+1 {{candidate}}
+  constexpr std::strong_ordering operator<=>(T y) const {
+return (x <=> y.x);
+  }
+};
+
+struct U {
+  int x;
+  // FIXME: This diagnostic is wrong-ish.
+  // expected-note@+1 {{candidate function not viable: requires single argument 'y', but 2 arguments were provided}}
+  constexpr std::strong_equality operator<=>(T y) const {
+if (x == y.x)
+  return std::strong_equality::equal;
+return std::strong_equality::nonequal;
+  }
+};
+
+struct X { int x; };
+struct Y { int x; };
+template 
+struct Tag {};
+// expected-note@+1 2 {{candidate}}
+Tag<0> operator<=>(X, Y) {
+  return {};
+}
+// expected-note@+1 2 {{candidate}}
+constexpr auto operator<=>(Y y, X x) {
+  return y.x <=> x.x;
+}
+
+void foo() {
+  T t{42};
+  T t2{0};
+  U u{101};
+  auto r1 = (t <=> u);
+  ASSERT_EXPR_TYPE(r1, std::strong_equality);
+  auto r2 = (t <=> t2);
+  ASSERT_EXPR_TYPE(r2, std::strong_ordering);
+
+  auto r3 = t == u;
+  ASSERT_EXPR_TYPE(r3, bool);
+
+  (void)(t < u); // expected-error {{invalid operands to binary expression ('TestRewritting::T' and 'TestRewritting::U')}}
+
+  constexpr X x{1};
+  constexpr Y y{2};
+  constexpr auto r4 = (y < x);
+  static_assert(r4 == false);
+  constexpr auto r5 = (x < y);
+  static_assert(r5 == true);
+}
+
+} // namespace TestRewritting
+
+// The implicit object parameter is not considered when performing partial
+// ordering. That makes the reverse synthesized candidates ambiguous with the
+// rewritten candidates if any of them resolve to a member function.
+namespace TestOvlMatchingIgnoresImplicitObject {
+struct U;
+// expected-note@+2 {{candidate}}
+struct T {
+  std::strong_ordering operator<=>(U const &RHS) const;
+};
+// expected-note@+2 {{candidate}}
+struct U {
+  std::strong_ordering operator<=>(T const &RHS) const;
+};
+
+void test() {
+  // expected-error@+1 {{use of overloaded operator '<' is ambiguous}}
+  (void)(T{} < U{});
+}
+
+} // namespace TestOvlMatchingIgnoresImplicitObject
+
+namespace TestImplicitConversionAmbiguity {
+struct T {
+  int x;
+};
+auto operator<=>(T const &LHS, T &&RHS) {
+  return LHS.x <=> RHS.x;
+}
+auto operator<(T const &, T &&) {
+  return std::strong_equality::equal;
+}
+void test() {
+  {
+// Chooses non-rewritten operator<
+  

[PATCH] D46441: [clang][CodeGenCXX] Noalias attr for copy/move constructor arguments

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

rsmith requested changes to this revision.
rsmith added a comment.
This revision now requires changes to proceed.

The code you're generating is not correct. The object being constructed cannot 
be accessed through a pointer not derived from `this`, but for a copy or move 
constructor, it is perfectly legitimate for the //source// of the construction 
to be aliased. For example:

  extern struct A a;
  struct A {
int m, n;
A(const A &v) : m(v.m), n((a.m = 1, v.m)) {}
  };

It would be incorrect to optimize this constructor to

  A(const A &v) : m(v.m), n(v.m) { a.m = 1; }

by deducing that `v` cannot alias `::a`, but it appears that is exactly what 
your patch will allow.

So: you should mark the `this` parameter as `noalias`, not the formal parameter 
of the function. (And as Eli notes, you should not treat the copy / move 
constructors as being special cases.)

However, I would question whether this change is correct at all. Note that the 
standard wording says "the value of the object or subobject thus obtained is 
unspecified" not "the behavior is undefined", and as far as I can tell, a 
violation of `noalias` results in undefined behavior in LLVM IR, not merely an 
unspecified value. (For what it's worth, I think this standard text is in error 
and UB would be appropriate, but we should at least discuss whether we want to 
take this additional liberty not technically granted by the standard.)




Comment at: lib/CodeGen/CodeGenModule.cpp:2513
+  cast(D)->isCopyOrMoveConstructor())
+F->addParamAttr(1, llvm::Attribute::NoAlias);
+

It's not strictly correct to assume that you know the correspondence between 
`llvm::Function` parameter indices and those of the `clang::FunctionDecl` like 
this. That's up to the ABI, and for constructors in particular, there may be a 
VTT parameter to deal with (and `__attribute__((pass_object_size))` can also 
interfere). You can probably get away with assuming that the `this` parameter 
has index 0 for now, but even that is likely to break at some point down the 
line.


Repository:
  rC Clang

https://reviews.llvm.org/D46441



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

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

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

rsmith added inline comments.



Comment at: lib/AST/ExprConstant.cpp:8829
+  return EvaluateComparisonBinaryOperator(Info, E, OnSuccess, [&]() {
+return ExprEvaluatorBaseTy::VisitBinaryOperator(E);
+  });

EricWF wrote:
> rsmith wrote:
> > It'd be clearer to call `VisitBinCmp` rather than `VisitBinaryOperator`.
> @rsmith: OK, so I'm confused about this. Originally I had an 
> `llvm_unreachable` that the continuation was never reached, but you suggested 
> it was. I'm not sure how. Could you provide an example?
> 
> The precondition of calling `VisitBinCmp` is that we have a call to a builtin 
> operator. For `<=>`,  where the composite type is either an arithmetic type, 
> pointer type, or member pointer type (which includes enum types after 
> conversions),  *All* of these cases should be handled before reaching the 
> function.
> 
> Is there a control flow path I'm missing? 
What about comparisons of `_Complex` types, vector types, and any other builtin 
type that gets added after you commit this patch? The right thing to do (at 
least for now) in all of those cases is to call the base class implementation, 
which will deal with emitting the "sorry, I don't know how to constant-evaluate 
this" diagnostic.

My comment here was simply that when doing so, you should call the base-class 
version of the *same* function, which you now do, so that concern is addressed.


https://reviews.llvm.org/D45476



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

[PATCH] D46176: Add SourceManagerForFile helper which sets up SourceManager and dependencies for a single file with code snippet

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 145386.
ioeric added a comment.

- Replaced forward decl of DiagnosticsEngine with header include.


Repository:
  rC Clang

https://reviews.llvm.org/D46176

Files:
  include/clang/Basic/SourceManager.h
  lib/Basic/SourceManager.cpp
  lib/Format/Format.cpp
  lib/Format/SortJavaScriptImports.cpp
  lib/Format/TokenAnalyzer.cpp
  lib/Format/TokenAnalyzer.h

Index: lib/Format/TokenAnalyzer.h
===
--- lib/Format/TokenAnalyzer.h
+++ lib/Format/TokenAnalyzer.h
@@ -37,44 +37,24 @@
 class Environment {
 public:
   Environment(SourceManager &SM, FileID ID, ArrayRef Ranges)
-  : ID(ID), CharRanges(Ranges.begin(), Ranges.end()), SM(SM),
-  FirstStartColumn(0),
-  NextStartColumn(0),
-  LastStartColumn(0) {}
-
-  Environment(FileID ID, std::unique_ptr FileMgr,
-  std::unique_ptr VirtualSM,
-  std::unique_ptr Diagnostics,
-  const std::vector &CharRanges,
-  unsigned FirstStartColumn,
-  unsigned NextStartColumn,
-  unsigned LastStartColumn)
-  : ID(ID), CharRanges(CharRanges.begin(), CharRanges.end()),
-SM(*VirtualSM), 
-FirstStartColumn(FirstStartColumn),
-NextStartColumn(NextStartColumn),
-LastStartColumn(LastStartColumn),
-FileMgr(std::move(FileMgr)),
-VirtualSM(std::move(VirtualSM)), Diagnostics(std::move(Diagnostics)) {}
+  : SM(SM), ID(ID), CharRanges(Ranges.begin(), Ranges.end()),
+FirstStartColumn(0), NextStartColumn(0), LastStartColumn(0) {}
 
   // This sets up an virtual file system with file \p FileName containing the
   // fragment \p Code. Assumes that \p Code starts at \p FirstStartColumn,
   // that the next lines of \p Code should start at \p NextStartColumn, and
   // that \p Code should end at \p LastStartColumn if it ends in newline.
   // See also the documentation of clang::format::internal::reformat.
-  static std::unique_ptr
-  CreateVirtualEnvironment(StringRef Code, StringRef FileName,
-   ArrayRef Ranges,
-   unsigned FirstStartColumn = 0,
-   unsigned NextStartColumn = 0,
-   unsigned LastStartColumn = 0);
+  Environment(StringRef Code, StringRef FileName,
+  ArrayRef Ranges, unsigned FirstStartColumn = 0,
+  unsigned NextStartColumn = 0, unsigned LastStartColumn = 0);
 
   FileID getFileID() const { return ID; }
 
-  ArrayRef getCharRanges() const { return CharRanges; }
-
   const SourceManager &getSourceManager() const { return SM; }
 
+  ArrayRef getCharRanges() const { return CharRanges; }
+
   // Returns the column at which the fragment of code managed by this
   // environment starts.
   unsigned getFirstStartColumn() const { return FirstStartColumn; }
@@ -88,19 +68,17 @@
   unsigned getLastStartColumn() const { return LastStartColumn; }
 
 private:
+  std::unique_ptr VirtualSM;
+
+  // This refers to either a SourceManager provided by users or VirtualSM
+  // created for a single file.
+  SourceManager &SM;
   FileID ID;
+
   SmallVector CharRanges;
-  SourceManager &SM;
   unsigned FirstStartColumn;
   unsigned NextStartColumn;
   unsigned LastStartColumn;
-
-  // The order of these fields are important - they should be in the same order
-  // as they are created in `CreateVirtualEnvironment` so that they can be
-  // deleted in the reverse order as they are created.
-  std::unique_ptr FileMgr;
-  std::unique_ptr VirtualSM;
-  std::unique_ptr Diagnostics;
 };
 
 class TokenAnalyzer : public UnwrappedLineConsumer {
Index: lib/Format/TokenAnalyzer.cpp
===
--- lib/Format/TokenAnalyzer.cpp
+++ lib/Format/TokenAnalyzer.cpp
@@ -34,48 +34,21 @@
 namespace clang {
 namespace format {
 
-// This sets up an virtual file system with file \p FileName containing \p
-// Code.
-std::unique_ptr
-Environment::CreateVirtualEnvironment(StringRef Code, StringRef FileName,
-  ArrayRef Ranges,
-  unsigned FirstStartColumn,
-  unsigned NextStartColumn,
-  unsigned LastStartColumn) {
-  // This is referenced by `FileMgr` and will be released by `FileMgr` when it
-  // is deleted.
-  IntrusiveRefCntPtr InMemoryFileSystem(
-  new vfs::InMemoryFileSystem);
-  // This is passed to `SM` as reference, so the pointer has to be referenced
-  // in `Environment` so that `FileMgr` can out-live this function scope.
-  std::unique_ptr FileMgr(
-  new FileManager(FileSystemOptions(), InMemoryFileSystem));
-  // This is passed to `SM` as reference, so the pointer has to be referenced
-  // by `Environment` due to the same reason above.
-  std::unique_ptr Diagnostics(new DiagnosticsEngine(
-  IntrusiveRefCntPtr(new DiagnosticIDs),
-  new

[PATCH] D46497: [clangd] Populate #include insertions as additional edits in completion items.

[Eric Liu via Phabricator via cfe-commits]

ioeric created this revision.
ioeric added a reviewer: sammccall.
Herald added subscribers: cfe-commits, jkorous, MaskRay, ilya-biryukov, klimek.

o Remove IncludeInsertion LSP command.
o Populate include insertion edits synchromously in completion items.
o Share the code completion compiler instance and precompiled preamble to get

  existing inclusions in main file.

o Include insertion logic lives only in CodeComplete now.
o Use tooling::HeaderIncludes for inserting new includes.
o Refactored tests.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46497

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  clangd/Headers.cpp
  clangd/Headers.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  clangd/XRefs.cpp
  clangd/tool/ClangdMain.cpp
  test/clangd/initialize-params-invalid.test
  test/clangd/initialize-params.test
  test/clangd/insert-include.test
  unittests/clangd/ClangdTests.cpp
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/HeadersTests.cpp

Index: unittests/clangd/HeadersTests.cpp
===
--- unittests/clangd/HeadersTests.cpp
+++ unittests/clangd/HeadersTests.cpp
@@ -8,7 +8,13 @@
 //===--===//
 
 #include "Headers.h"
+
+#include "Compiler.h"
 #include "TestFS.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Lex/PreprocessorOptions.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -29,17 +35,48 @@
 bool ExpectError = false) {
 if (Preferred.empty())
   Preferred = Original;
-auto VFS = FS.getFileSystem();
+
 auto Cmd = CDB.getCompileCommand(MainFile);
 assert(static_cast(Cmd));
+auto VFS = FS.getFileSystem();
 VFS->setCurrentWorkingDirectory(Cmd->Directory);
+
+std::vector Argv;
+for (const auto &S : Cmd->CommandLine)
+  Argv.push_back(S.c_str());
+IgnoringDiagConsumer IgnoreDiags;
+auto CI = clang::createInvocationFromCommandLine(
+Argv,
+CompilerInstance::createDiagnostics(new DiagnosticOptions(),
+&IgnoreDiags, false),
+VFS);
+EXPECT_TRUE(static_cast(CI));
+CI->getFrontendOpts().DisableFree = false;
+
+// The diagnostic options must be set before creating a CompilerInstance.
+CI->getDiagnosticOpts().IgnoreWarnings = true;
+auto Clang = prepareCompilerInstance(
+std::move(CI), /*Preamble=*/nullptr,
+llvm::MemoryBuffer::getMemBuffer(FS.Files[MainFile], MainFile),
+std::make_shared(), VFS, IgnoreDiags);
+
+EXPECT_FALSE(Clang->getFrontendOpts().Inputs.empty());
+PreprocessOnlyAction Action;
+EXPECT_TRUE(
+Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0]));
+
+IncludePaths Paths(MainFile, Cmd->Directory, Clang->getSourceManager(),
+   /*InclusionLocations*/ {}, Clang->getPreprocessorPtr());
+EXPECT_TRUE(Action.Execute());
+Action.EndSourceFile();
+
 auto ToHeaderFile = [](llvm::StringRef Header) {
   return HeaderFile{Header,
 /*Verbatim=*/!llvm::sys::path::is_absolute(Header)};
 };
-auto Path = calculateIncludePath(MainFile, FS.Files[MainFile],
- ToHeaderFile(Original),
- ToHeaderFile(Preferred), *Cmd, VFS);
+
+auto Path =
+Paths.calculate(ToHeaderFile(Original), ToHeaderFile(Preferred));
 if (!Path) {
   llvm::consumeError(Path.takeError());
   EXPECT_TRUE(ExpectError);
@@ -49,6 +86,7 @@
 }
 return std::move(*Path);
   }
+
   MockFSProvider FS;
   MockCompilationDatabase CDB;
   std::string MainFile = testPath("main.cpp");
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -45,6 +45,16 @@
 MATCHER_P(Filter, F, "") { return arg.filterText == F; }
 MATCHER_P(Doc, D, "") { return arg.documentation == D; }
 MATCHER_P(Detail, D, "") { return arg.detail == D; }
+MATCHER_P(InsertInclude, IncludeHeader, "") {
+  if (arg.additionalTextEdits.size() != 1)
+return false;
+  const auto &Edit = arg.additionalTextEdits[0];
+  if (Edit.range.start != Edit.range.end)
+return false;
+  SmallVector Matches;
+  llvm::Regex RE(R"(#include[ ]*(["<][^">]*[">]))");
+  return RE.match(Edit.newText, &Matches) && Matches[1] == IncludeHeader;
+}
 MATCHER_P(PlainText, Text, "") {
   return arg.insertTextFormat == clangd::InsertTextFormat::PlainText &&
  arg.insertText == Text;
@@ -58,6 +68,8 @@
 return true;
   return llvm::S

[PATCH] D46496: [Tooling] Pull #include manipulation code from clangFormat into libToolingCore.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 145384.
ioeric added a comment.

- Revert unintended change.


Repository:
  rC Clang

https://reviews.llvm.org/D46496

Files:
  include/clang/Format/Format.h
  include/clang/Tooling/Core/HeaderIncludes.h
  lib/Format/Format.cpp
  lib/Tooling/Core/CMakeLists.txt
  lib/Tooling/Core/HeaderIncludes.cpp
  unittests/Format/CleanupTest.cpp
  unittests/Format/FormatTest.cpp
  unittests/Format/SortIncludesTest.cpp
  unittests/Tooling/CMakeLists.txt
  unittests/Tooling/HeaderIncludesTest.cpp

Index: unittests/Tooling/HeaderIncludesTest.cpp
===
--- /dev/null
+++ unittests/Tooling/HeaderIncludesTest.cpp
@@ -0,0 +1,527 @@
+//===- unittest/Tooling/CleanupTest.cpp - Include insertion/deletion tests ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/Tooling/Core/HeaderIncludes.h"
+#include "../Tooling/ReplacementTest.h"
+#include "../Tooling/RewriterTestContext.h"
+#include "clang/Format/Format.h"
+#include "clang/Tooling/Core/Replacement.h"
+
+#include "gtest/gtest.h"
+
+using clang::tooling::ReplacementTest;
+using clang::tooling::toReplacements;
+
+namespace clang {
+namespace tooling {
+namespace {
+
+class HeaderIncludesTest : public ::testing::Test {
+protected:
+  std::string insert(llvm::StringRef Code, llvm::StringRef Header) {
+HeaderIncludes Includes(FileName, Code, Style);
+assert(Header.startswith("\"") || Header.startswith("<"));
+auto R = Includes.insert(Header.trim("\"<>"), Header.startswith("<"));
+if (!R)
+  return Code;
+auto Result = applyAllReplacements(Code, Replacements(*R));
+EXPECT_TRUE(static_cast(Result));
+return *Result;
+  }
+
+  std::string remove(llvm::StringRef Code, llvm::StringRef Header) {
+HeaderIncludes Includes(FileName, Code, Style);
+assert(Header.startswith("\"") || Header.startswith("<"));
+auto Replaces = Includes.remove(Header.trim("\"<>"), Header.startswith("<"));
+auto Result = applyAllReplacements(Code, Replaces);
+EXPECT_TRUE(static_cast(Result));
+return *Result;
+  }
+
+  const std::string FileName = "fix.cpp";
+  IncludeStyle Style = format::getLLVMStyle().IncludeStyle;
+};
+
+TEST_F(HeaderIncludesTest, NoExistingIncludeWithoutDefine) {
+  std::string Code = "int main() {}";
+  std::string Expected = "#include \"a.h\"\n"
+ "int main() {}";
+  EXPECT_EQ(Expected, insert(Code, "\"a.h\""));
+}
+
+TEST_F(HeaderIncludesTest, NoExistingIncludeWithDefine) {
+  std::string Code = "#ifndef A_H\n"
+ "#define A_H\n"
+ "class A {};\n"
+ "#define MMM 123\n"
+ "#endif";
+  std::string Expected = "#ifndef A_H\n"
+ "#define A_H\n"
+ "#include \"b.h\"\n"
+ "class A {};\n"
+ "#define MMM 123\n"
+ "#endif";
+
+  EXPECT_EQ(Expected, insert(Code, "\"b.h\""));
+}
+
+TEST_F(HeaderIncludesTest, InsertBeforeCategoryWithLowerPriority) {
+  std::string Code = "#ifndef A_H\n"
+ "#define A_H\n"
+ "\n"
+ "\n"
+ "\n"
+ "#include \n"
+ "class A {};\n"
+ "#define MMM 123\n"
+ "#endif";
+  std::string Expected = "#ifndef A_H\n"
+ "#define A_H\n"
+ "\n"
+ "\n"
+ "\n"
+ "#include \"a.h\"\n"
+ "#include \n"
+ "class A {};\n"
+ "#define MMM 123\n"
+ "#endif";
+
+  EXPECT_EQ(Expected, insert(Code, "\"a.h\""));
+}
+
+TEST_F(HeaderIncludesTest, InsertAfterMainHeader) {
+  std::string Code = "#include \"fix.h\"\n"
+ "\n"
+ "int main() {}";
+  std::string Expected = "#include \"fix.h\"\n"
+ "#include \n"
+ "\n"
+ "int main() {}";
+  Style = format::getGoogleStyle(format::FormatStyle::LanguageKind::LK_Cpp)
+  .IncludeStyle;
+  EXPECT_EQ(Expected, insert(Code, ""));
+}
+
+TEST_F(HeaderIncludesTest, InsertBeforeSystemHeaderLLVM) {
+  std::string Code = "#include \n"
+ "\n"
+ "int main() {}";
+  std::string Expected = "#include \"z.h\"\n"
+ "#include \n"
+ "\n"
+ "int main() {}";
+  EXPECT_EQ(Expected, insert(Code, "\"z.h\""));
+}
+
+TEST_F(HeaderIncludesTest, InsertAfterSystemHeaderGoogle) {
+  std::strin

[PATCH] D46496: [Tooling] Pull #include manipulation code from clangFormat into libToolingCore.

[Eric Liu via Phabricator via cfe-commits]

ioeric created this revision.
ioeric added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, mgorny, klimek.

Also pull #include related style out of FormatStyle as tooling::IncludeStyle.


Repository:
  rC Clang

https://reviews.llvm.org/D46496

Files:
  include/clang/Basic/SourceManager.h
  include/clang/Format/Format.h
  include/clang/Tooling/Core/HeaderIncludes.h
  lib/Format/Format.cpp
  lib/Tooling/Core/CMakeLists.txt
  lib/Tooling/Core/HeaderIncludes.cpp
  unittests/Format/CleanupTest.cpp
  unittests/Format/FormatTest.cpp
  unittests/Format/SortIncludesTest.cpp
  unittests/Tooling/CMakeLists.txt
  unittests/Tooling/HeaderIncludesTest.cpp

Index: unittests/Tooling/HeaderIncludesTest.cpp
===
--- /dev/null
+++ unittests/Tooling/HeaderIncludesTest.cpp
@@ -0,0 +1,527 @@
+//===- unittest/Tooling/CleanupTest.cpp - Include insertion/deletion tests ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/Tooling/Core/HeaderIncludes.h"
+#include "../Tooling/ReplacementTest.h"
+#include "../Tooling/RewriterTestContext.h"
+#include "clang/Format/Format.h"
+#include "clang/Tooling/Core/Replacement.h"
+
+#include "gtest/gtest.h"
+
+using clang::tooling::ReplacementTest;
+using clang::tooling::toReplacements;
+
+namespace clang {
+namespace tooling {
+namespace {
+
+class HeaderIncludesTest : public ::testing::Test {
+protected:
+  std::string insert(llvm::StringRef Code, llvm::StringRef Header) {
+HeaderIncludes Includes(FileName, Code, Style);
+assert(Header.startswith("\"") || Header.startswith("<"));
+auto R = Includes.insert(Header.trim("\"<>"), Header.startswith("<"));
+if (!R)
+  return Code;
+auto Result = applyAllReplacements(Code, Replacements(*R));
+EXPECT_TRUE(static_cast(Result));
+return *Result;
+  }
+
+  std::string remove(llvm::StringRef Code, llvm::StringRef Header) {
+HeaderIncludes Includes(FileName, Code, Style);
+assert(Header.startswith("\"") || Header.startswith("<"));
+auto Replaces = Includes.remove(Header.trim("\"<>"), Header.startswith("<"));
+auto Result = applyAllReplacements(Code, Replaces);
+EXPECT_TRUE(static_cast(Result));
+return *Result;
+  }
+
+  const std::string FileName = "fix.cpp";
+  IncludeStyle Style = format::getLLVMStyle().IncludeStyle;
+};
+
+TEST_F(HeaderIncludesTest, NoExistingIncludeWithoutDefine) {
+  std::string Code = "int main() {}";
+  std::string Expected = "#include \"a.h\"\n"
+ "int main() {}";
+  EXPECT_EQ(Expected, insert(Code, "\"a.h\""));
+}
+
+TEST_F(HeaderIncludesTest, NoExistingIncludeWithDefine) {
+  std::string Code = "#ifndef A_H\n"
+ "#define A_H\n"
+ "class A {};\n"
+ "#define MMM 123\n"
+ "#endif";
+  std::string Expected = "#ifndef A_H\n"
+ "#define A_H\n"
+ "#include \"b.h\"\n"
+ "class A {};\n"
+ "#define MMM 123\n"
+ "#endif";
+
+  EXPECT_EQ(Expected, insert(Code, "\"b.h\""));
+}
+
+TEST_F(HeaderIncludesTest, InsertBeforeCategoryWithLowerPriority) {
+  std::string Code = "#ifndef A_H\n"
+ "#define A_H\n"
+ "\n"
+ "\n"
+ "\n"
+ "#include \n"
+ "class A {};\n"
+ "#define MMM 123\n"
+ "#endif";
+  std::string Expected = "#ifndef A_H\n"
+ "#define A_H\n"
+ "\n"
+ "\n"
+ "\n"
+ "#include \"a.h\"\n"
+ "#include \n"
+ "class A {};\n"
+ "#define MMM 123\n"
+ "#endif";
+
+  EXPECT_EQ(Expected, insert(Code, "\"a.h\""));
+}
+
+TEST_F(HeaderIncludesTest, InsertAfterMainHeader) {
+  std::string Code = "#include \"fix.h\"\n"
+ "\n"
+ "int main() {}";
+  std::string Expected = "#include \"fix.h\"\n"
+ "#include \n"
+ "\n"
+ "int main() {}";
+  Style = format::getGoogleStyle(format::FormatStyle::LanguageKind::LK_Cpp)
+  .IncludeStyle;
+  EXPECT_EQ(Expected, insert(Code, ""));
+}
+
+TEST_F(HeaderIncludesTest, InsertBeforeSystemHeaderLLVM) {
+  std::string Code = "#include \n"
+ "\n"
+ "int main() {}";
+  std::string Expected = "#include \"z.h\"\n"
+ "#include \n"
+ "\n"
+ "

OpenCL Header: Guard all half float usage based on cl_khr_fp16 extension

[via cfe-commits]

Hi,
 
this patch guards all missed half float functions based on the availabiltiy of 
the OpenCL extension cl_khr_fp16.
 
Best regards,
 
JerryIndex: lib/Headers/opencl-c.h
===
--- lib/Headers/opencl-c.h	(revision 331598)
+++ lib/Headers/opencl-c.h	(working copy)
@@ -6668,12 +6668,12 @@
 // OpenCL v1.1 s6.9, v1.2/2.0 s6.10 - Function qualifiers
 
 #define __kernel_exec(X, typen) __kernel \
-	__attribute__((work_group_size_hint(X, 1, 1))) \
-	__attribute__((vec_type_hint(typen)))
+__attribute__((work_group_size_hint(X, 1, 1))) \
+__attribute__((vec_type_hint(typen)))
 
 #define kernel_exec(X, typen) __kernel \
-	__attribute__((work_group_size_hint(X, 1, 1))) \
-	__attribute__((vec_type_hint(typen)))
+__attribute__((work_group_size_hint(X, 1, 1))) \
+__attribute__((vec_type_hint(typen)))
 
 // OpenCL v1.1 s6.11.1, v1.2 s6.12.1, v2.0 s6.13.1 - Work-item Functions
 
@@ -11540,7 +11540,7 @@
  *
  * vstoren write sizeof (gentypen) bytes given by data to address (p + (offset * n)).
  *
- * The address computed as (p + (offset * n)) must be 
+ * The address computed as (p + (offset * n)) must be
  * 8-bit aligned if gentype is char, uchar;
  * 16-bit aligned if gentype is short, ushort, half;
  * 32-bit aligned if gentype is int, uint, float;
@@ -12093,6 +12093,7 @@
  * The read address computed as (p + offset)
  * must be 16-bit aligned.
  */
+#ifdef cl_khr_fp16
 float __ovld vload_half(size_t offset, const __constant half *p);
 #if __OPENCL_C_VERSION__ >= CL_VERSION_2_0
 float __ovld vload_half(size_t offset, const half *p);
@@ -12101,6 +12102,7 @@
 float __ovld vload_half(size_t offset, const __local half *p);
 float __ovld vload_half(size_t offset, const __private half *p);
 #endif //__OPENCL_C_VERSION__ >= CL_VERSION_2_0
+#endif //cl_khr_fp16
 
 /**
  * Read sizeof (halfn) bytes of data from address
@@ -12110,6 +12112,7 @@
  * value is returned. The read address computed
  * as (p + (offset * n)) must be 16-bit aligned.
  */
+#ifdef cl_khr_fp16
 float2 __ovld vload_half2(size_t offset, const __constant half *p);
 float3 __ovld vload_half3(size_t offset, const __constant half *p);
 float4 __ovld vload_half4(size_t offset, const __constant half *p);
@@ -12138,6 +12141,7 @@
 float8 __ovld vload_half8(size_t offset, const __private half *p);
 float16 __ovld vload_half16(size_t offset, const __private half *p);
 #endif //__OPENCL_C_VERSION__ >= CL_VERSION_2_0
+#endif //cl_khr_fp16
 
 /**
  * The float value given by data is first
@@ -12150,6 +12154,7 @@
  * The default current rounding mode is round to
  * nearest even.
  */
+#ifdef cl_khr_fp16
 #if __OPENCL_C_VERSION__ >= CL_VERSION_2_0
 void __ovld vstore_half(float data, size_t offset, half *p);
 void __ovld vstore_half_rte(float data, size_t offset, half *p);
@@ -12197,6 +12202,7 @@
 void __ovld vstore_half_rtn(double data, size_t offset, __private half *p);
 #endif //cl_khr_fp64
 #endif //__OPENCL_C_VERSION__ >= CL_VERSION_2_0
+#endif //cl_khr_fp16
 
 /**
  * The floatn value given by data is converted to
@@ -12209,6 +12215,7 @@
  * The default current rounding mode is round to
  * nearest even.
  */
+#ifdef cl_khr_fp16
 #if __OPENCL_C_VERSION__ >= CL_VERSION_2_0
 void __ovld vstore_half2(float2 data, size_t offset, half *p);
 void __ovld vstore_half3(float3 data, size_t offset, half *p);
@@ -12416,6 +12423,7 @@
 void __ovld vstore_half16_rtn(double16 data, size_t offset, __private half *p);
 #endif //cl_khr_fp64
 #endif //__OPENCL_C_VERSION__ >= CL_VERSION_2_0
+#endif //cl_khr_fp16
 
 /**
  * For n = 1, 2, 4, 8 and 16 read sizeof (halfn)
@@ -12430,6 +12438,7 @@
  * The address computed as (p + (offset * 4))
  * must be aligned to sizeof (half) * 4 bytes.
  */
+#ifdef cl_khr_fp16
 float __ovld vloada_half(size_t offset, const __constant half *p);
 float2 __ovld vloada_half2(size_t offset, const __constant half *p);
 float3 __ovld vloada_half3(size_t offset, const __constant half *p);
@@ -12463,6 +12472,7 @@
 float8 __ovld vloada_half8(size_t offset, const __private half *p);
 float16 __ovld vloada_half16(size_t offset, const __private half *p);
 #endif //__OPENCL_C_VERSION__ >= CL_VERSION_2_0
+#endif //cl_khr_fp16
 
 /**
  * The floatn value given by data is converted to
@@ -12480,6 +12490,7 @@
  * mode. The default current rounding mode is
  * round to nearest even.
  */
+#ifdef cl_khr_fp16
 #if __OPENCL_C_VERSION__ >= CL_VERSION_2_0
 void __ovld vstorea_half(float data, size_t offset, half *p);
 void __ovld vstorea_half2(float2 data, size_t offset, half *p);
@@ -12766,6 +12777,7 @@
 void __ovld vstorea_half16_rtn(double16 data,size_t offset, __private half *p);
 #endif //cl_khr_fp64
 #endif //__OPENCL_C_VERSION__ >= CL_VERSION_2_0
+#endif //cl_khr_fp16
 
 // OpenCL v1.1 s6.11.8, v1.2 s6.12.8, v2.0 s6.13.8 - Synchronization Functions
 
@@ -12888,7 +12900,7 @@
 cl_mem_fence_flags __ovld get_fence(const void *ptr);
 cl_mem_fence_flags __ovld get_fence(void *ptr);
 

[PATCH] D45927: [clang-tidy] [modernize-use-auto] Correct way to calculate a type name length for multi-token types

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis added inline comments.



Comment at: clang-tidy/modernize-use-auto-min-type-name-length.cpp:61-83
+long int li = static_cast(foo());
+// CHECK-FIXES-0-0: auto li = {{.*}}
+// CHECK-FIXES-0-5: auto li = {{.*}}
+// CHECK-FIXES-1-0: auto  li = {{.*}}
+// CHECK-FIXES-1-5: auto  li = {{.*}}
+long int *pli = static_cast(foo());
+// CHECK-FIXES-0-0: auto *pli = {{.*}}

alexfh wrote:
> These tests could be more useful, if they verified boundary values of the 
> MinTypeNameLength, e.g. that `long int` is replaced with `auto` when the 
> option is set to 8 and it stays `long int`with the option set to 9.
> 
> Please also add tests with template typenames: e.g. the lenght of `T <  int  
> >` should be considered 6 and the length of `T  <  const int >` is 12. I 
> guess, the algorithm I proposed will be incorrect for pointer type arguments 
> of templates (the length of `T` should be 7 regardless of 
> `RemoveStars`), but this can be fixed by conditionally (on RemoveStars) 
> trimming `*`s from the right of the type name and then treating them as 
> punctuation. So instead of
> 
> ```
>   for (const unsigned char C : tooling::fixit::getText(SR, Context)) {
> const CharType NextChar =
> std::isalnum(C)
> ? Alpha
> : (std::isspace(C) || (!RemoveStars && C == '*')) ? Space
>   : Punctuation;
> ```
> 
> you could use something similar to
> 
> ```
>   StringRef Text = tooling::fixit::getText(SR, Context);
>   if (RemoveStars)
> Text = Text.rtrim(" \t\v\n\r*");
>   for (const unsigned char C : Text) {
> const CharType NextChar =
> std::isalnum(C) ? Alpha : std::isspace(C) ? Space : Punctuation;
> ```
> These tests could be more useful, if they verified boundary values of the 
> MinTypeNameLength, e.g. that long int is replaced with auto when the option 
> is set to 8 and it stays long intwith the option set to 9.
> 

`int`-test is just for that :-)

```
int a = static_cast(foo()); // ---> int  a = ...
```


https://reviews.llvm.org/D45927



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

[PATCH] D45944: Disallow pointers to const in __sync_fetch_and_xxx

[Aaron Ballman via Phabricator via cfe-commits]

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

Committed in r331598, with an additional test case.

(Accepting officially on behalf of @rjmccall so that I can close the review.)


https://reviews.llvm.org/D45944



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

r331598 - Disallow pointers to const in __sync_fetch_and_xxx.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Sat May  5 10:38:42 2018
New Revision: 331598

URL: http://llvm.org/viewvc/llvm-project?rev=331598&view=rev
Log:
Disallow pointers to const in __sync_fetch_and_xxx.

Diagnoses code like:

void f(const int *ptr) {
  __sync_fetch_and_add(ptr, 1);
}

which matches the behavior of GCC and ICC.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/Sema/builtins.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=331598&r1=331597&r2=331598&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Sat May  5 10:38:42 
2018
@@ -7094,6 +7094,8 @@ def err_atomic_builtin_must_be_pointer :
 def err_atomic_builtin_must_be_pointer_intptr : Error<
   "address argument to atomic builtin must be a pointer to integer or pointer"
   " (%0 invalid)">;
+def err_atomic_builtin_cannot_be_const : Error<
+  "address argument to atomic builtin cannot be const-qualified (%0 invalid)">;
 def err_atomic_builtin_must_be_pointer_intfltptr : Error<
   "address argument to atomic builtin must be a pointer to integer,"
   " floating-point or pointer (%0 invalid)">;

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=331598&r1=331597&r2=331598&view=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Sat May  5 10:38:42 2018
@@ -3445,6 +3445,12 @@ Sema::SemaBuiltinAtomicOverloaded(ExprRe
 return ExprError();
   }
 
+  if (ValType.isConstQualified()) {
+Diag(DRE->getLocStart(), diag::err_atomic_builtin_cannot_be_const)
+<< FirstArg->getType() << FirstArg->getSourceRange();
+return ExprError();
+  }
+
   switch (ValType.getObjCLifetime()) {
   case Qualifiers::OCL_None:
   case Qualifiers::OCL_ExplicitNone:

Modified: cfe/trunk/test/Sema/builtins.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/builtins.c?rev=331598&r1=331597&r2=331598&view=diff
==
--- cfe/trunk/test/Sema/builtins.c (original)
+++ cfe/trunk/test/Sema/builtins.c Sat May  5 10:38:42 2018
@@ -248,3 +248,8 @@ char * Test20(char *p, const char *in, u
 
 return buf;
 }
+
+void test21(const int *ptr) {
+  __sync_fetch_and_add(ptr, 1); // expected-error{{address argument to atomic 
builtin cannot be const-qualified ('const int *' invalid)}}
+  __atomic_fetch_add(ptr, 1, 0);  // expected-error {{address argument to 
atomic operation must be a pointer to non-const type ('const int *' invalid)}}
+}


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

r331597 - Add -target to address errors in test from r331592

[Teresa Johnson via cfe-commits]

Author: tejohnson
Date: Sat May  5 09:37:31 2018
New Revision: 331597

URL: http://llvm.org/viewvc/llvm-project?rev=331597&view=rev
Log:
Add -target to address errors in test from r331592

The error turns out to be:
Assertion failed: (Target.isCompatibleDataLayout(getDataLayout()) && "Can't 
create a MachineFunction using a Module with a " "Target-incompatible 
DataLayout attached\n"), function init, file 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/lib/CodeGen/MachineFunction.cpp,
 line 180.

Add -target to address this. Also re-enable the test I had temporarily
commented, and move it further down in case there is still a failure
(since it pipes stderr to FileCheck).

Modified:
cfe/trunk/test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll

Modified: 
cfe/trunk/test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll?rev=331597&r1=331596&r2=331597&view=diff
==
--- cfe/trunk/test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll 
(original)
+++ cfe/trunk/test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll 
Sat May  5 09:37:31 2018
@@ -5,15 +5,9 @@
 ; RUN: opt -module-summary -o %t.o %s
 ; RUN: llvm-lto -thinlto -o %t %t.o
 
-; First try with pass remarks to stderr
-; Temporarily skip the next RUN/CHECK to debug bot failures
-; RUN %clang -O2 -x ir %t.o -fthinlto-index=%t.thinlto.bc -mllvm 
-pass-remarks=inline -fdiagnostics-show-hotness -o %t2.o -c 2>&1 | FileCheck %s
-
-; CHECK tinkywinky inlined into main with cost=0 (threshold=337) (hotness: 300)
-
-; Next try YAML pass remarks file
+; First try YAML pass remarks file
 ; RUN: rm -f %t2.opt.yaml
-; RUN: %clang -O2 -x ir %t.o -fthinlto-index=%t.thinlto.bc 
-fsave-optimization-record -fdiagnostics-show-hotness -o %t2.o -c
+; RUN: %clang -target x86_64-scei-ps4 -O2 -x ir %t.o 
-fthinlto-index=%t.thinlto.bc -fsave-optimization-record 
-fdiagnostics-show-hotness -o %t2.o -c
 ; RUN: cat %t2.opt.yaml | FileCheck %s -check-prefix=YAML
 
 ; YAML: --- !Passed
@@ -32,6 +26,11 @@
 ; YAML-NEXT:   - String:  ')'
 ; YAML-NEXT: ...
 
+; Next try with pass remarks to stderr
+; RUN: %clang -target x86_64-scei-ps4 -O2 -x ir %t.o 
-fthinlto-index=%t.thinlto.bc -mllvm -pass-remarks=inline 
-fdiagnostics-show-hotness -o %t2.o -c 2>&1 | FileCheck %s
+
+; CHECK: tinkywinky inlined into main with cost=0 (threshold=337) (hotness: 
300)
+
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-scei-ps4"
 


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

[PATCH] D45679: [clang-tidy] Add ExprMutationAnalyzer, that analyzes whether an expression is mutated within a statement.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D45679#1088696, @shuaiwang wrote:

> I've reverted the handling of DeclRefExpr to volatile after rethinking about 
> it.
>
> The purpose of this analyzer is to find whether the given Expr is mutated 
> within a scope, but doesn't really care about external changes. In other 
> words we're trying to find mutations explicitly written within the specified 
> scope.


Okay, that seems like a good reason to ignore volatile qualifiers. Thank you 
for the explanation!




Comment at: clang-tidy/utils/ExprMutationAnalyzer.cpp:88
+
+const Stmt *ExprMutationAnalyzer::findDirectMutation(const Expr *Exp) {
+  // LHS of any assignment operators.

shuaiwang wrote:
> aaron.ballman wrote:
> > aaron.ballman wrote:
> > > JonasToth wrote:
> > > > shuaiwang wrote:
> > > > > aaron.ballman wrote:
> > > > > > Should this also consider a DeclRefExpr to a volatile-qualified 
> > > > > > variable as a direct mutation?
> > > > > > 
> > > > > > What about using `Expr::HasSideEffect()`?
> > > > > Good catch about DeclRefExpr to volatile.
> > > > > 
> > > > > `HasSideEffects` means something different. Here find*Mutation means 
> > > > > find a Stmt **in ancestors** that mutates the given Expr. 
> > > > > `HasSideEffects` IIUC means whether anything is mutated by the Expr 
> > > > > but doesn't care about what exactly is mutated.
> > > > May I ask the exact semantics for `volatile`? I would add the test to 
> > > > my check, too.
> > > > 
> > > > It is possible to write `const volatile int m = 42;`, but if i 
> > > > understand correctly `m` could still change by hardware, or would this 
> > > > be UB?
> > > > If the `const` is missing, one must always assume external 
> > > > modification, right?
> > > > HasSideEffects means something different. Here find*Mutation means find 
> > > > a Stmt in ancestors that mutates the given Expr. HasSideEffects IIUC 
> > > > means whether anything is mutated by the Expr but doesn't care about 
> > > > what exactly is mutated.
> > > 
> > > What I am getting at is that the code in `findDirectMutation()` seems to 
> > > go through a lot of effort looking for expressions that have side effects 
> > > and I was wondering if we could leverage that call instead of duplicating 
> > > the effort.
> > > May I ask the exact semantics for volatile? I would add the test to my 
> > > check, too.
> > 
> > Basically, volatile objects can change in ways unknown to the 
> > implementation, so a volatile read must always perform an actual read 
> > (because the object's value may have changed since the last read) and a 
> > volatile write must always be performed as well (because writing a value 
> > may have further side effects if the object is backed by some piece of 
> > interesting hardware).
> > 
> > > It is possible to write const volatile int m = 42;, but if i understand 
> > > correctly m could still change by hardware, or would this be UB?
> > 
> > That could still be changed by hardware, and it would not be UB. For 
> > instance, the variable might be referencing some hardware sensor and so you 
> > can only read the values in (hence it's const) but the values may be 
> > changed out from under you (hence, it's not constant).
> > 
> > > If the const is missing, one must always assume external modification, 
> > > right?
> > 
> > You have to assume external modification regardless.
> Unfortunately I don't think HasSideEffects help in here. We're finding one 
> particular side effect that is mutating the specified Expr so we need to have 
> explicitly `equalsNode(Exp)` for all the matches. `HasSideEffects` can simply 
> claim `f(a, b)` or even `g()` has side effects (given `void f(Foo&, const 
> Bar&)`, `void g()`) while we need to require a link to the specific Expr 
> we're analyzing (i.e. `f(a, b)` is mutating `a` but not `b`)
Ah, okay, thank you for the explanation.

I think you may want to look at the implementation for `HasSideEffect()` to 
identify other cases you may want to support. For instance, adding tests for 
statement expressions, paren expressions, bizarre things like VLA mutations in 
otherwise unevaluated contexts, etc.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45679



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

r331596 - Skip part of test added in r331592 to help debug bot failures

[Teresa Johnson via cfe-commits]

Author: tejohnson
Date: Sat May  5 08:54:57 2018
New Revision: 331596

URL: http://llvm.org/viewvc/llvm-project?rev=331596&view=rev
Log:
Skip part of test added in r331592 to help debug bot failures

Trying to debug why/where a few bots getting exit code 256 e.g.
http://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/48471/testReport/Clang/CodeGen/thinlto_diagnostic_handler_remarks_with_hotness_ll/

and a few windows bots getting no output from that RUN line e.g.
http://lab.llvm.org:8011/builders/clang-x86-windows-msvc2015/builds/11865/steps/ninja%20check%201/logs/FAIL%3A%20Clang%3A%3Athinlto-diagnostic-handler-remarks-with-hotness.ll

Modified:
cfe/trunk/test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll

Modified: 
cfe/trunk/test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll?rev=331596&r1=331595&r2=331596&view=diff
==
--- cfe/trunk/test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll 
(original)
+++ cfe/trunk/test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll 
Sat May  5 08:54:57 2018
@@ -6,9 +6,10 @@
 ; RUN: llvm-lto -thinlto -o %t %t.o
 
 ; First try with pass remarks to stderr
-; RUN: %clang -O2 -x ir %t.o -fthinlto-index=%t.thinlto.bc -mllvm 
-pass-remarks=inline -fdiagnostics-show-hotness -o %t2.o -c 2>&1 | FileCheck %s
+; Temporarily skip the next RUN/CHECK to debug bot failures
+; RUN %clang -O2 -x ir %t.o -fthinlto-index=%t.thinlto.bc -mllvm 
-pass-remarks=inline -fdiagnostics-show-hotness -o %t2.o -c 2>&1 | FileCheck %s
 
-; CHECK: tinkywinky inlined into main with cost=0 (threshold=337) (hotness: 
300)
+; CHECK tinkywinky inlined into main with cost=0 (threshold=337) (hotness: 300)
 
 ; Next try YAML pass remarks file
 ; RUN: rm -f %t2.opt.yaml


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

r331593 - Add required target to address bot failures from r331592

[Teresa Johnson via cfe-commits]

Author: tejohnson
Date: Sat May  5 08:15:04 2018
New Revision: 331593

URL: http://llvm.org/viewvc/llvm-project?rev=331593&view=rev
Log:
Add required target to address bot failures from r331592

Failing on non-x86 bots, needs x86 target for code gen.

Modified:
cfe/trunk/test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll

Modified: 
cfe/trunk/test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll?rev=331593&r1=331592&r2=331593&view=diff
==
--- cfe/trunk/test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll 
(original)
+++ cfe/trunk/test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll 
Sat May  5 08:15:04 2018
@@ -1,5 +1,7 @@
 ; Test to ensure -fdiagnostics-show-hotness and -fsave-optimization-record
 ; work when invoking the ThinLTO backend path.
+; REQUIRES: x86-registered-target
+
 ; RUN: opt -module-summary -o %t.o %s
 ; RUN: llvm-lto -thinlto -o %t %t.o
 


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

[PATCH] D46464: [ThinLTO] Support opt remarks options with distributed ThinLTO backends

[Teresa Johnson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL331592: [ThinLTO] Support opt remarks options with 
distributed ThinLTO backends (authored by tejohnson, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D46464

Files:
  cfe/trunk/lib/CodeGen/BackendUtil.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll
  cfe/trunk/test/CodeGen/thinlto_backend.ll

Index: cfe/trunk/test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll
===
--- cfe/trunk/test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll
+++ cfe/trunk/test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll
@@ -0,0 +1,47 @@
+; Test to ensure -fdiagnostics-show-hotness and -fsave-optimization-record
+; work when invoking the ThinLTO backend path.
+; RUN: opt -module-summary -o %t.o %s
+; RUN: llvm-lto -thinlto -o %t %t.o
+
+; First try with pass remarks to stderr
+; RUN: %clang -O2 -x ir %t.o -fthinlto-index=%t.thinlto.bc -mllvm -pass-remarks=inline -fdiagnostics-show-hotness -o %t2.o -c 2>&1 | FileCheck %s
+
+; CHECK: tinkywinky inlined into main with cost=0 (threshold=337) (hotness: 300)
+
+; Next try YAML pass remarks file
+; RUN: rm -f %t2.opt.yaml
+; RUN: %clang -O2 -x ir %t.o -fthinlto-index=%t.thinlto.bc -fsave-optimization-record -fdiagnostics-show-hotness -o %t2.o -c
+; RUN: cat %t2.opt.yaml | FileCheck %s -check-prefix=YAML
+
+; YAML: --- !Passed
+; YAML-NEXT: Pass:inline
+; YAML-NEXT: Name:Inlined
+; YAML-NEXT: Function:main
+; YAML-NEXT: Hotness: 300
+; YAML-NEXT: Args:
+; YAML-NEXT:   - Callee:  tinkywinky
+; YAML-NEXT:   - String:  ' inlined into '
+; YAML-NEXT:   - Caller:  main
+; YAML-NEXT:   - String:  ' with cost='
+; YAML-NEXT:   - Cost:'0'
+; YAML-NEXT:   - String:  ' (threshold='
+; YAML-NEXT:   - Threshold:   '337'
+; YAML-NEXT:   - String:  ')'
+; YAML-NEXT: ...
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-scei-ps4"
+
+declare i32 @patatino()
+
+define i32 @tinkywinky() {
+  %a = call i32 @patatino()
+  ret i32 %a
+}
+
+define i32 @main() !prof !0 {
+  %i = call i32 @tinkywinky()
+  ret i32 %i
+}
+
+!0 = !{!"function_entry_count", i64 300}
Index: cfe/trunk/test/CodeGen/thinlto_backend.ll
===
--- cfe/trunk/test/CodeGen/thinlto_backend.ll
+++ cfe/trunk/test/CodeGen/thinlto_backend.ll
@@ -29,13 +29,13 @@
 
 ; Ensure f2 was imported. Check for all 3 flavors of -save-temps[=cwd|obj].
 ; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc -save-temps=obj
-; RUN: llvm-dis %t1.s.0.3.import.bc -o - | FileCheck --check-prefix=CHECK-IMPORT %s
+; RUN: llvm-dis %t1.s.3.import.bc -o - | FileCheck --check-prefix=CHECK-IMPORT %s
 ; RUN: mkdir -p %T/dir1
 ; RUN: (cd %T/dir1 && %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc -save-temps=cwd)
-; RUN: llvm-dis %T/dir1/*1.s.0.3.import.bc -o - | FileCheck --check-prefix=CHECK-IMPORT %s
+; RUN: llvm-dis %T/dir1/*1.s.3.import.bc -o - | FileCheck --check-prefix=CHECK-IMPORT %s
 ; RUN: mkdir -p %T/dir2
 ; RUN: (cd %T/dir2 && %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc -save-temps)
-; RUN: llvm-dis %T/dir2/*1.s.0.3.import.bc -o - | FileCheck --check-prefix=CHECK-IMPORT %s
+; RUN: llvm-dis %T/dir2/*1.s.3.import.bc -o - | FileCheck --check-prefix=CHECK-IMPORT %s
 ; CHECK-IMPORT: define available_externally void @f2()
 ; RUN: llvm-nm %t3.o | FileCheck --check-prefix=CHECK-OBJ %s
 ; CHECK-OBJ: T f1
Index: cfe/trunk/lib/CodeGen/BackendUtil.cpp
===
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp
@@ -1136,6 +1136,8 @@
   Conf.SampleProfile = std::move(SampleProfile);
   Conf.UseNewPM = CGOpts.ExperimentalNewPassManager;
   Conf.DebugPassManager = CGOpts.DebugPassManager;
+  Conf.RemarksWithHotness = CGOpts.DiagnosticsWithHotness;
+  Conf.RemarksFilename = CGOpts.OptRecordFile;
   switch (Action) {
   case Backend_EmitNothing:
 Conf.PreCodeGenModuleHook = [](size_t Task, const Module &Mod) {
@@ -1159,7 +1161,7 @@
 break;
   }
   if (Error E = thinBackend(
-  Conf, 0, AddStream, *M, *CombinedIndex, ImportList,
+  Conf, -1, AddStream, *M, *CombinedIndex, ImportList,
   ModuleToDefinedGVSummaries[M->getModuleIdentifier()], ModuleMap)) {
 handleAllErrors(std::move(E), [&](ErrorInfoBase &EIB) {
   errs() << "Error running ThinLTO backend: " << EIB.message() << '\n';
Index: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
=

r331592 - [ThinLTO] Support opt remarks options with distributed ThinLTO backends

[Teresa Johnson via cfe-commits]

Author: tejohnson
Date: Sat May  5 07:37:29 2018
New Revision: 331592

URL: http://llvm.org/viewvc/llvm-project?rev=331592&view=rev
Log:
[ThinLTO] Support opt remarks options with distributed ThinLTO backends

Summary:
Passes down the necessary code ge options to the LTO Config to enable
-fdiagnostics-show-hotness and -fsave-optimization-record in the ThinLTO
backend for a distributed build.

Also, remove warning about not having PGO when the input is IR.

Reviewers: pcc

Subscribers: mehdi_amini, inglorion, eraman, cfe-commits

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

Added:
cfe/trunk/test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll
Modified:
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/CodeGen/thinlto_backend.ll

Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=331592&r1=331591&r2=331592&view=diff
==
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Sat May  5 07:37:29 2018
@@ -1136,6 +1136,8 @@ static void runThinLTOBackend(ModuleSumm
   Conf.SampleProfile = std::move(SampleProfile);
   Conf.UseNewPM = CGOpts.ExperimentalNewPassManager;
   Conf.DebugPassManager = CGOpts.DebugPassManager;
+  Conf.RemarksWithHotness = CGOpts.DiagnosticsWithHotness;
+  Conf.RemarksFilename = CGOpts.OptRecordFile;
   switch (Action) {
   case Backend_EmitNothing:
 Conf.PreCodeGenModuleHook = [](size_t Task, const Module &Mod) {
@@ -1159,7 +1161,7 @@ static void runThinLTOBackend(ModuleSumm
 break;
   }
   if (Error E = thinBackend(
-  Conf, 0, AddStream, *M, *CombinedIndex, ImportList,
+  Conf, -1, AddStream, *M, *CombinedIndex, ImportList,
   ModuleToDefinedGVSummaries[M->getModuleIdentifier()], ModuleMap)) {
 handleAllErrors(std::move(E), [&](ErrorInfoBase &EIB) {
   errs() << "Error running ThinLTO backend: " << EIB.message() << '\n';

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=331592&r1=331591&r2=331592&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Sat May  5 07:37:29 2018
@@ -1075,7 +1075,9 @@ static bool ParseCodeGenArgs(CodeGenOpti
   bool UsingProfile = UsingSampleProfile ||
   (Opts.getProfileUse() != CodeGenOptions::ProfileNone);
 
-  if (Opts.DiagnosticsWithHotness && !UsingProfile)
+  if (Opts.DiagnosticsWithHotness && !UsingProfile &&
+  // An IR file will contain PGO as metadata
+  IK.getLanguage() != InputKind::LLVM_IR)
 Diags.Report(diag::warn_drv_diagnostics_hotness_requires_pgo)
 << "-fdiagnostics-show-hotness";
 

Added: cfe/trunk/test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll?rev=331592&view=auto
==
--- cfe/trunk/test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll 
(added)
+++ cfe/trunk/test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll 
Sat May  5 07:37:29 2018
@@ -0,0 +1,47 @@
+; Test to ensure -fdiagnostics-show-hotness and -fsave-optimization-record
+; work when invoking the ThinLTO backend path.
+; RUN: opt -module-summary -o %t.o %s
+; RUN: llvm-lto -thinlto -o %t %t.o
+
+; First try with pass remarks to stderr
+; RUN: %clang -O2 -x ir %t.o -fthinlto-index=%t.thinlto.bc -mllvm 
-pass-remarks=inline -fdiagnostics-show-hotness -o %t2.o -c 2>&1 | FileCheck %s
+
+; CHECK: tinkywinky inlined into main with cost=0 (threshold=337) (hotness: 
300)
+
+; Next try YAML pass remarks file
+; RUN: rm -f %t2.opt.yaml
+; RUN: %clang -O2 -x ir %t.o -fthinlto-index=%t.thinlto.bc 
-fsave-optimization-record -fdiagnostics-show-hotness -o %t2.o -c
+; RUN: cat %t2.opt.yaml | FileCheck %s -check-prefix=YAML
+
+; YAML: --- !Passed
+; YAML-NEXT: Pass:inline
+; YAML-NEXT: Name:Inlined
+; YAML-NEXT: Function:main
+; YAML-NEXT: Hotness: 300
+; YAML-NEXT: Args:
+; YAML-NEXT:   - Callee:  tinkywinky
+; YAML-NEXT:   - String:  ' inlined into '
+; YAML-NEXT:   - Caller:  main
+; YAML-NEXT:   - String:  ' with cost='
+; YAML-NEXT:   - Cost:'0'
+; YAML-NEXT:   - String:  ' (threshold='
+; YAML-NEXT:   - Threshold:   '337'
+; YAML-NEXT:   - String:  ')'
+; YAML-NEXT: ...
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-scei-ps4"
+
+declare i32 @patatino()
+
+define i32 @tinkywinky() {
+  %a = call i32 @patatino()
+  ret i32 %a
+}
+
+define i32 @mai

[PATCH] D46450: [Driver] Add mips_Features_Group to Options to improve documentation sorting

[Simon Atanasyan via Phabricator via cfe-commits]

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

LGTM

Do you have commit access?


Repository:
  rC Clang

https://reviews.llvm.org/D46450



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

[PATCH] D46233: [ASTMatchers] Overload isConstexpr for ifStmts

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

xazax.hun marked 2 inline comments as done.
xazax.hun added inline comments.



Comment at: docs/LibASTMatchersReference.html:179
 
-MatcherDecl>cxxMethodDeclMatcherCXXMethodDecl>...
+MatcherDecl>cxxMethodDeclMatcher...
 Matches method 
declarations.

alexfh wrote:
> alexfh wrote:
> > The changes don't look nice. It looks like either dump_ast_matchers.py 
> > can't access network on your machine. Can this be the case? The URLs (e.g. 
> > http://clang.llvm.org/doxygen/classclang_1_1CXXMethodDecl.html) seem to 
> > work for me.
> Just checked that the script still works fine.
Thanks! I think this is the result of my or the doxygen host's network being 
unreliable. I was unable to generate a good documentation after the first few 
trials, the links for a few (but not all) node were always missing. I ended up 
pruning those changes from the diff manually.


https://reviews.llvm.org/D46233



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

[PATCH] D46233: [ASTMatchers] Overload isConstexpr for ifStmts

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

xazax.hun updated this revision to Diff 145370.
xazax.hun added a comment.

- Rerun script


https://reviews.llvm.org/D46233

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h
  unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -894,6 +894,10 @@
   varDecl(hasName("foo"), isConstexpr(;
   EXPECT_TRUE(matches("constexpr int bar();",
   functionDecl(hasName("bar"), isConstexpr(;
+  EXPECT_TRUE(matchesConditionally("void baz() { if constexpr(1 > 0) {} }",
+   ifStmt(isConstexpr()), true, "-std=c++17"));
+  EXPECT_TRUE(matchesConditionally("void baz() { if (1 > 0) {} }",
+   ifStmt(isConstexpr()), false, "-std=c++17"));
 }
 
 TEST(TemplateArgumentCountIs, Matches) {
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -3763,20 +3763,25 @@
   return FnTy->isNothrow(Finder->getASTContext());
 }
 
-/// \brief Matches constexpr variable and function declarations.
+/// \brief Matches constexpr variable and function declarations,
+///and if constexpr.
 ///
 /// Given:
 /// \code
 ///   constexpr int foo = 42;
 ///   constexpr int bar();
+///   void baz() { if constexpr(1 > 0) {} }
 /// \endcode
 /// varDecl(isConstexpr())
 ///   matches the declaration of foo.
 /// functionDecl(isConstexpr())
 ///   matches the declaration of bar.
+/// ifStmt(isConstexpr())
+///   matches the if statement in baz.
 AST_POLYMORPHIC_MATCHER(isConstexpr,
 AST_POLYMORPHIC_SUPPORTED_TYPES(VarDecl,
-FunctionDecl)) {
+FunctionDecl,
+IfStmt)) {
   return Node.isConstexpr();
 }
 
Index: docs/LibASTMatchersReference.html
===
--- docs/LibASTMatchersReference.html
+++ docs/LibASTMatchersReference.html
@@ -1926,15 +1926,16 @@
 
 
 
-MatcherBinaryOperator>isAssignmentOperator
-Matches all kinds of assignment operators.
+MatcherBinaryOperator>isAssignmentOperator
+Matches on all kinds of assignment operators.
 
 Example 1: matches a += b (matcher = binaryOperator(isAssignmentOperator()))
   if (a == b)
 a += b;
 
-Example 2: matches s1 = s2 (matcher = cxxOperatorCallExpr(isAssignmentOperator()))
-  struct S { S& operator=(const S&); };
+Example 2: matches s1 = s2
+   (matcher = cxxOperatorCallExpr(isAssignmentOperator()))
+  struct S { S& operator=(const S&); };
   void x() { S s1, s2; s1 = s2; })
 
 
@@ -2319,15 +2320,16 @@
 
 
 
-MatcherCXXOperatorCallExpr>isAssignmentOperator
-Matches all kinds of assignment operators.
+MatcherCXXOperatorCallExpr>isAssignmentOperator
+Matches on all kinds of assignment operators.
 
 Example 1: matches a += b (matcher = binaryOperator(isAssignmentOperator()))
   if (a == b)
 a += b;
 
-Example 2: matches s1 = s2 (matcher = cxxOperatorCallExpr(isAssignmentOperator()))
-  struct S { S& operator=(const S&); };
+Example 2: matches s1 = s2
+   (matcher = cxxOperatorCallExpr(isAssignmentOperator()))
+  struct S { S& operator=(const S&); };
   void x() { S s1, s2; s1 = s2; })
 
 
@@ -2787,15 +2789,19 @@
 
 
 MatcherFunctionDecl>isConstexpr
-Matches constexpr variable and function declarations.
+Matches constexpr variable and function declarations,
+   and if constexpr.
 
 Given:
   constexpr int foo = 42;
   constexpr int bar();
+  void baz() { if constexpr(1 > 0) {} }
 varDecl(isConstexpr())
   matches the declaration of foo.
 functionDecl(isConstexpr())
   matches the declaration of bar.
+ifStmt(isConstexpr())
+  matches the if statement in baz.
 
 
 
@@ -3037,6 +3043,23 @@
 
 
 
+MatcherIfStmt>isConstexpr
+Matches constexpr variable and function declarations,
+   and if constexpr.
+
+Given:
+  constexpr int foo = 42;
+  constexpr int bar();
+  void baz() { if constexpr(1 > 0) {} }
+varDecl(isConstexpr())
+  matches the declaration of foo.
+functionDecl(isConstexpr())
+  matches the declaration of bar.
+ifStmt(isConstexpr())
+  matches the if statement in baz.
+
+
+
 Matcher

[PATCH] D46187: [Analyzer] getRegisteredCheckers(): handle debug checkers too.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D46187#1088722, @NoQ wrote:

> It seems that you're using debug checkers of the analyzer to gain some free 
> tools for exploring the source code (such as displaying a call graph) for 
> free, right?


Yes.
I was interested to dump the callgraph of `DAGCombiner.cpp`, and try to analyze 
if there is any sane way to split it up.
I think it would require a bit of changes to that callgraph dumper (writing LOC 
count for the functions that actually exist-as-written (or something) in the 
source code, grouping the functions in the main TU into a one group), but i did 
not get to that.

> I believe we could also benefit from a method of extracting the analyzer's 
> `clang -cc1` run-line from clang-tidy. This would allow arbitrary debugging 
> over a single analyzer invocation.

I'm not really following, sorry.


Repository:
  rC Clang

https://reviews.llvm.org/D46187



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

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added inline comments.



Comment at: lib/AST/ExprConstant.cpp:8829
+  return EvaluateComparisonBinaryOperator(Info, E, OnSuccess, [&]() {
+return ExprEvaluatorBaseTy::VisitBinaryOperator(E);
+  });

rsmith wrote:
> It'd be clearer to call `VisitBinCmp` rather than `VisitBinaryOperator`.
@rsmith: OK, so I'm confused about this. Originally I had an `llvm_unreachable` 
that the continuation was never reached, but you suggested it was. I'm not sure 
how. Could you provide an example?

The precondition of calling `VisitBinCmp` is that we have a call to a builtin 
operator. For `<=>`,  where the composite type is either an arithmetic type, 
pointer type, or member pointer type (which includes enum types after 
conversions),  *All* of these cases should be handled before reaching the 
function.

Is there a control flow path I'm missing? 


https://reviews.llvm.org/D45476



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

[PATCH] D46441: [clang][CodeGenCXX] Noalias attr for copy/move constructor arguments

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

@efriedma copy and move constructors are particular cases where the value of a 
constructed object may be accessed through a glvalue not obtained from 'this' 
pointer (but from the first arg of a ctor).


Repository:
  rC Clang

https://reviews.llvm.org/D46441



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

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

[Henry Wong via Phabricator via cfe-commits]

MTC updated this revision to Diff 145361.
MTC added a comment.

- Since there is no perfect way to handle the default binding of non-zero 
character, remove the default binding of non-zero character. Use 
`bindDefaulrZero()` instead of `overwriteRegion()` to bind the zero character.
- Reuse `assume()` instead of `isZeroConstant()` to determine whether it is 
zero character. The purpose of this is to be able to set the string length 
**when dealing with non-zero symbol character**.


Repository:
  rC Clang

https://reviews.llvm.org/D44934

Files:
  lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  test/Analysis/bstring.cpp
  test/Analysis/null-deref-ps-region.c
  test/Analysis/string.c

Index: test/Analysis/string.c
===
--- test/Analysis/string.c
+++ test/Analysis/string.c
@@ -1,7 +1,8 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.cstring,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -Wno-null-dereference -verify %s
-// RUN: %clang_analyze_cc1 -DUSE_BUILTINS -analyzer-checker=core,unix.cstring,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -Wno-null-dereference -verify %s
-// RUN: %clang_analyze_cc1 -DVARIANT -analyzer-checker=core,unix.cstring,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -Wno-null-dereference -verify %s
-// RUN: %clang_analyze_cc1 -DUSE_BUILTINS -DVARIANT -analyzer-checker=alpha.security.taint,core,unix.cstring,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -Wno-null-dereference -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.cstring,unix.Malloc,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -Wno-null-dereference -verify %s
+// RUN: %clang_analyze_cc1 -DUSE_BUILTINS -analyzer-checker=core,unix.cstring,unix.Malloc,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -Wno-null-dereference -verify %s
+// RUN: %clang_analyze_cc1 -DVARIANT -analyzer-checker=core,unix.cstring,unix.Malloc,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -Wno-null-dereference -verify %s
+// RUN: %clang_analyze_cc1 -DUSE_BUILTINS -DVARIANT -analyzer-checker=alpha.security.taint,core,unix.cstring,unix.Malloc,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -Wno-null-dereference -verify %s
+// RUN: %clang_analyze_cc1 -DSUPPRESS_OUT_OF_BOUND -analyzer-checker=core,unix.cstring,unix.Malloc,alpha.unix.cstring.BufferOverlap,alpha.unix.cstring.NotNullTerminated,debug.ExprInspection -analyzer-store=region -Wno-null-dereference -verify %s
 
 //===--===
 // Declarations
@@ -1159,6 +1160,186 @@
   clang_analyzer_eval(str[1] == 'b'); // expected-warning{{UNKNOWN}}
 }
 
+//===--===
+// memset()
+//===--===
+
+void *memset(void *dest, int ch, size_t count);
+
+void *malloc(size_t size);
+void free(void *);
+
+void memset1_char_array_null() {
+  char str[] = "abcd";
+  clang_analyzer_eval(strlen(str) == 4); // expected-warning{{TRUE}}
+  memset(str, '\0', 2);
+  clang_analyzer_eval(strlen(str) == 0); // expected-warning{{TRUE}}
+}
+
+void memset2_char_array_null() {
+  char str[] = "abcd";
+  clang_analyzer_eval(strlen(str) == 4); // expected-warning{{TRUE}}
+  memset(str, '\0', strlen(str) + 1);
+  clang_analyzer_eval(strlen(str) == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(str[2] == 0);  // expected-warning{{TRUE}}
+}
+
+void memset3_char_malloc_null() {
+  char *str = (char *)malloc(10 * sizeof(char));
+  memset(str + 1, '\0', 8);
+  clang_analyzer_eval(str[1] == 0); // expected-warning{{UNKNOWN}}
+  free(str);
+}
+
+void memset4_char_malloc_null() {
+  char *str = (char *)malloc(10 * sizeof(char));
+  //void *str = malloc(10 * sizeof(char));
+  memset(str, '\0', 10);
+  clang_analyzer_eval(str[1] == 0);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(strlen(str) == 0); // expected-warning{{TRUE}}
+  free(str);
+}
+
+#ifdef SUPPRESS_OUT_OF_BOUND
+void memset5_char_malloc_overflow_null() {
+  char *str = (char *)malloc(10 * sizeof(char));
+  memset(str, '\0', 12);
+  clang_analyzer_eval(str[1] == 0); // expected-warning{{UNKNOWN}}
+  free(str);
+}
+#endif
+
+void memset6_char_array_nonnull() {
+  char str[] = "abcd";
+  clang_analyzer_eval(strlen(str) == 4); // expected-warning{{TRUE}}
+  memset(str, '0', 2);
+  clang_analyzer_eval(str[0] == 'a');// expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(strlen(str) == 4); // expected-warning{{UNKNOWN}}
+}
+
+#ifdef SUPPRESS_OUT_OF_BOUND
+void memset8_char_array_nonnull() {
+  char str[5] = "abcd";
+  clang_analyzer_eval(strlen(str) == 4); // expected-warning{{TRUE}}
+  memset(str, '0', 10);
+  clang_analyzer_eval(str[0] != '0'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(strlen(str) >= 10); // expected-warning{{TRUE}}
+  clang

[PATCH] D46155: Add warning flag -Wordered-compare-function-pointers.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Ideally there should be a test that verifies that 
`-Wordered-compare-function-pointers` / 
`-Wno-ordered-compare-function-pointers` / the default is what you expect it to 
be.


Repository:
  rC Clang

https://reviews.llvm.org/D46155



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

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added inline comments.



Comment at: lib/CodeGen/CGExprAgg.cpp:964
+RHS = CGF.EmitAnyExpr(E->getRHS()).getAggregatePointer();
+break;
+  case TEK_Complex:

EricWF wrote:
> rjmccall wrote:
> > It looks like we don't actually support any aggregate types here, which I 
> > think is fine because comparing those types is only sensible for things 
> > like calls.  If you do want to pave the way for that, or (probably more 
> > usefully) for supporting complex types, you should make EmitCompare take 
> > the operands as RValues and just use EmitAnyExpr here without paying any 
> > attention to the evaluation kind.
> Initially I thought the same thing, but apparently member pointers are 
> Aggregates under the Microsoft ABI.
> 
> I'll give  trafficking in `RValue`s, but all the functions `EmitCompare` 
> calls use `Value*`, so it'll take some work.
*I'll give trafficking in `RValue`s a shot, but ...*


https://reviews.llvm.org/D45476



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

[PATCH] D45774: [analyzer] cover more cases where a Loc can be bound to constants

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

xazax.hun added inline comments.



Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1642-1645
+// Return unknown value if index is out of bounds.
+if (i < 0 || i >= InitList->getNumInits()) {
+  return UnknownVal();
+}

NoQ wrote:
> Hmm, we might as well want to return an `UndefinedVal()` when the index is 
> out of bounds of the actual array. The size of the array can be retrieved 
> from `VD`. Though if you decide to do that, please put it into a separate 
> patch :)
+1 for the UndefVal patch :)


Repository:
  rC Clang

https://reviews.llvm.org/D45774



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

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

[Eric Fiselier via Phabricator via cfe-commits]

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



Comment at: include/clang/AST/ComparisonCategories.h:71
+  /// standard library. The key is a value of ComparisonCategoryResult.
+  mutable llvm::DenseMap Objects;
+

rjmccall wrote:
> EricWF wrote:
> > rjmccall wrote:
> > > We expect this map to have at least two of the seven result types, and 
> > > probably three or four, right?  It should probably just be an array; 
> > > it'll both be faster and use less memory.
> > > 
> > > (The similar map in `ComparisonCategories` is different because we expect 
> > > it to be empty in most translation units.)
> > Ack.
> > 
> > Do you want `std::array` or something slightly more conservative like 
> > `llvm::SmallVector`?
> std::array is definitely better here.
I went with `SmallVector`, because `ComparisonCategoryinfo` currently has the 
invariant that it always contains a valid `VarDecl`. When I implemented this 
using `std::array` removing that invariant made a lot of code more messy.





Comment at: lib/CodeGen/CGExprAgg.cpp:971
+  auto EmitCmpRes = [&](const VarDecl *VD) {
+return CGF.CGM.GetAddrOfGlobalVar(VD);
+  };

rjmccall wrote:
> EricWF wrote:
> > rjmccall wrote:
> > > EricWF wrote:
> > > > rsmith wrote:
> > > > > Perhaps directly emit the constant value here rather than the address 
> > > > > of the global? I think we should consider what IR we want to see 
> > > > > coming out of Clang, and I don't think that IR should contain loads 
> > > > > from globals to get the small constant integer that is the value of 
> > > > > the conversion result.
> > > > > 
> > > > > I think it would be reasonable for us to say that we require the 
> > > > > standard library types to contain exactly one non-static data member 
> > > > > of integral type, and for us to form a select between the relevant 
> > > > > integer values here. We really have no need to support all possible 
> > > > > implementations of these types, and we can revisit this if some other 
> > > > > standard library implementation ships types that don't follow that 
> > > > > pattern. (If we find such a standard library, we could emit multiple 
> > > > > selects, or a first-class aggregate select, or whatever generates the 
> > > > > best code at -O0.)
> > > > I agree emitting the value would be better, and that most STL 
> > > > implementations should implement the types using only one non-static 
> > > > member.
> > > > However, note that the specification for `partial_ordering` is 
> > > > described in terms of two non-static data members, so it seems possible 
> > > > an STL implementation might implement in that way.
> > > > 
> > > > Would it be appropriate to do this as a smaller follow up patch?
> > > While it would be nice if we could special-case the case of a class with 
> > > a single integral field all the way through the various uses of it, IRGen 
> > > is not at all set up to try to take advantage of that.  You will need to 
> > > store your integral result into the dest slot here anyway.  That makes me 
> > > suspect that there's just no value in trying to produce a scalar select 
> > > before doing that store instead of branching to pick one of two stores.
> > > 
> > > Also, I know there's this whole clever argument for why we can get away 
> > > with lazily finding this comparison-result type and its static members in 
> > > translation units that are just deserializing a spaceship operator.  Just 
> > > to make me feel better, though, please actually check here dynamically 
> > > that the assumptions we're relying on actually hold and that we've found 
> > > an appropriate variable for the comparison result and it does have an 
> > > initializer.  It is fine to generate an atrocious diagnostic if we see a 
> > > failure, but let's please not crash just because something weird and 
> > > unexpected happened with module import.
> > > While it would be nice if we could special-case the case of a class with 
> > > a single integral field all the way through the various uses of it, IRGen 
> > > is not at all set up to try to take advantage of that. You will need to 
> > > store your integral result into the dest slot here anyway. That makes me 
> > > suspect that there's just no value in trying to produce a scalar select 
> > > before doing that store instead of branching to pick one of two stores.
> > 
> > I went ahead and did it anyway, though I suspect you might be right. I'll 
> > need to look into it further. (In particular if we avoid ODR uses and hence 
> > can avoid emitting the inline variable definitions).
> > 
> > > Just to make me feel better, though, please actually check here 
> > > dynamically that the assumptions we're relying on actually hold and that 
> > > we've found an appropriate variable for the comparison result and it does 
> > > have an initializer. 
> > 
> > Ack. I've already added checks in `Sema` that validate that the