[PATCH] D42459: [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D42459#2383753 , @ldionne wrote:

> @hintonda  Is there a reason why this was done except for "standardizing the 
> usage across projects"?

It was just for standardization.  Please feel free to rollback or modify as 
necessary.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D42459

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

[PATCH] D62445: [test] Fix plugin tests

[Don Hinton via Phabricator via cfe-commits]

hintonda marked an inline comment as done.
hintonda added inline comments.



Comment at: cfe/trunk/test/CMakeLists.txt:140
-  set_target_properties(check-clang-analyzer PROPERTIES FOLDER "Clang tests")
-
-  if (LLVM_WITH_Z3)

thakis wrote:
> Did you intentionally delete the 2 targets here? 
Not intentionally, although I don't remember the details right now.  Probably 
just a mistake on my part.

Do you want me to re-add them?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62445



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

[PATCH] D70259: [Error] Add source location to cantFail

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 230131.
hintonda added a comment.

- Add back original llvm::cantFail signatures so they'll still be


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70259

Files:
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/refactor/tweaks/ExpandMacro.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/FormatTests.cpp
  clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
  clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
  clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
  clang-tools-extra/clangd/unittests/TweakTesting.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp
  clang/lib/AST/ExternalASTMerger.cpp
  clang/lib/Tooling/Core/Replacement.cpp
  clang/lib/Tooling/Tooling.cpp
  clang/tools/clang-refactor/ClangRefactor.cpp
  clang/unittests/Basic/DiagnosticTest.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/CompileUnitIndex.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
  lldb/unittests/Target/ExecutionContextTest.cpp
  llvm/include/llvm/Support/Error.h
  llvm/unittests/Support/ErrorTest.cpp

Index: llvm/unittests/Support/ErrorTest.cpp
===
--- llvm/unittests/Support/ErrorTest.cpp
+++ llvm/unittests/Support/ErrorTest.cpp
@@ -390,8 +390,8 @@
   };
 
   EXPECT_DEATH(FailToHandle(),
-   "Failure value returned from cantFail wrapped call\n"
-   "CustomError \\{7\\}")
+   "CustomError \\{7\\}\n"
+   "Failure value returned from cantFail wrapped call")
   << "Unhandled Error in handleAllErrors call did not cause an "
  "abort()";
 }
@@ -410,8 +410,8 @@
   };
 
   EXPECT_DEATH(ReturnErrorFromHandler(),
-   "Failure value returned from cantFail wrapped call\n"
-   "CustomError \\{7\\}")
+   "CustomError \\{7\\}\n"
+   "Failure value returned from cantFail wrapped call")
   << " Error returned from handler in handleAllErrors call did not "
  "cause abort()";
 }
@@ -515,8 +515,8 @@
   EXPECT_DEATH(cantFail(make_error("Original error message",
 inconvertibleErrorCode()),
 "Cantfail call failed"),
-   "Cantfail call failed\n"
-   "Original error message")
+   "Original error message\n"
+   "Cantfail call failed")
   << "cantFail(Error) did not cause an abort for failure value";
 
   EXPECT_DEATH(
@@ -530,6 +530,24 @@
 }
 #endif
 
+TEST(Error, CantFailSourceLocation) {
+  std::string ErrStr;
+  raw_string_ostream OS(ErrStr);
+  OS << "\nFailure value returned from cantFail wrapped call";
+
+#if defined(NDEBUG)
+  // __FILE__ and __LINE_ not added
+  OS << "\n";
+  EXPECT_DEATH(cantFail(make_error(1)), OS.str())
+  << "cantFail(Error) did not cause an abort for failure value";
+#else
+  // __FILE__ and __LINE__ added
+  int Line = __LINE__;
+  OS << " at " << __FILE__ << ":" << (Line + 2) << "\n";
+  EXPECT_DEATH(cantFail(make_error(1)), OS.str())
+  << "cantFail(Error) did not cause an abort for failure value";
+#endif
+}
 
 // Test Checked Expected in success mode.
 TEST(Error, CheckedExpectedInSuccessMode) {
Index: llvm/include/llvm/Support/Error.h
===
--- llvm/include/llvm/Support/Error.h
+++ llvm/include/llvm/Support/Error.h
@@ -687,6 +687,48 @@
 LLVM_ATTRIBUTE_NORETURN void report_fatal_error(Error Err,
 bool gen_crash_diag = true);
 
+namespace detail {
+
+inline LLVM_ATTRIBUTE_NORETURN void handleError(Error Err, const char *Msg,
+const char *file = nullptr,
+unsigned line = 0) {
+  if (!Msg)
+Msg = "Failure value returned from cantFail wrapped call";
+#ifndef NDEBUG
+  std::string Str;
+  raw_string_ostream OS(Str);
+  OS << Err << "\n" << Msg;
+  if (file)
+OS << " at " << file << ":" << line;
+  Msg = OS.str().c_str();
+#endif
+  llvm_unreachable(Msg);
+}
+
+inline void cantFail(const char *file, unsigned line, Error Err,
+ const char *Msg = nullptr) {
+  if (Err)
+handleError(std::move(Err), Msg, file, line);
+}
+
+template 
+T cantFail(const char *file, unsigned line, Expected ValOrEr

[PATCH] D70259: [Error] Add source location to cantFail

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 230125.
hintonda added a comment.
Herald added subscribers: lldb-commits, cfe-commits, usaxena95, kadircet, 
arphaman, jkorous.
Herald added projects: clang, LLDB.

- Replace macro magic with matching 3-parameter template functions.
- Refactor cantFail move into detail namespace.  Change macro name to


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70259

Files:
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/refactor/tweaks/ExpandMacro.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/FormatTests.cpp
  clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
  clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
  clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
  clang-tools-extra/clangd/unittests/TweakTesting.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp
  clang/lib/AST/ExternalASTMerger.cpp
  clang/lib/Tooling/Core/Replacement.cpp
  clang/lib/Tooling/Tooling.cpp
  clang/tools/clang-refactor/ClangRefactor.cpp
  clang/unittests/Basic/DiagnosticTest.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/CompileUnitIndex.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
  lldb/unittests/Target/ExecutionContextTest.cpp
  llvm/include/llvm/Support/Error.h
  llvm/unittests/Support/ErrorTest.cpp

Index: llvm/unittests/Support/ErrorTest.cpp
===
--- llvm/unittests/Support/ErrorTest.cpp
+++ llvm/unittests/Support/ErrorTest.cpp
@@ -390,8 +390,8 @@
   };
 
   EXPECT_DEATH(FailToHandle(),
-   "Failure value returned from cantFail wrapped call\n"
-   "CustomError \\{7\\}")
+   "CustomError \\{7\\}\n"
+   "Failure value returned from cantFail wrapped call")
   << "Unhandled Error in handleAllErrors call did not cause an "
  "abort()";
 }
@@ -410,8 +410,8 @@
   };
 
   EXPECT_DEATH(ReturnErrorFromHandler(),
-   "Failure value returned from cantFail wrapped call\n"
-   "CustomError \\{7\\}")
+   "CustomError \\{7\\}\n"
+   "Failure value returned from cantFail wrapped call")
   << " Error returned from handler in handleAllErrors call did not "
  "cause abort()";
 }
@@ -515,8 +515,8 @@
   EXPECT_DEATH(cantFail(make_error("Original error message",
 inconvertibleErrorCode()),
 "Cantfail call failed"),
-   "Cantfail call failed\n"
-   "Original error message")
+   "Original error message\n"
+   "Cantfail call failed")
   << "cantFail(Error) did not cause an abort for failure value";
 
   EXPECT_DEATH(
@@ -530,6 +530,24 @@
 }
 #endif
 
+TEST(Error, CantFailSourceLocation) {
+  std::string ErrStr;
+  raw_string_ostream OS(ErrStr);
+  OS << "\nFailure value returned from cantFail wrapped call";
+
+#if defined(NDEBUG)
+  // __FILE__ and __LINE_ not added
+  OS << "\n";
+  EXPECT_DEATH(cantFail(make_error(1)), OS.str())
+  << "cantFail(Error) did not cause an abort for failure value";
+#else
+  // __FILE__ and __LINE__ added
+  int Line = __LINE__;
+  OS << " at " << __FILE__ << ":" << (Line + 2) << "\n";
+  EXPECT_DEATH(cantFail(make_error(1)), OS.str())
+  << "cantFail(Error) did not cause an abort for failure value";
+#endif
+}
 
 // Test Checked Expected in success mode.
 TEST(Error, CheckedExpectedInSuccessMode) {
Index: llvm/include/llvm/Support/Error.h
===
--- llvm/include/llvm/Support/Error.h
+++ llvm/include/llvm/Support/Error.h
@@ -687,6 +687,23 @@
 LLVM_ATTRIBUTE_NORETURN void report_fatal_error(Error Err,
 bool gen_crash_diag = true);
 
+namespace detail {
+
+inline LLVM_ATTRIBUTE_NORETURN void
+handleError(Error Err, const char *Msg, const char *file, unsigned line) {
+  if (!Msg)
+Msg = "Failure value returned from cantFail wrapped call";
+#ifndef NDEBUG
+  std::string Str;
+  raw_string_ostream OS(Str);
+  OS << Err << "\n" << Msg;
+  if (file)
+OS << " at " << file << ":" << line;
+  Msg = OS.str().c_str();
+#endif
+  llvm_unreachable(Msg);
+}
+
 /// Report a fatal error if Err is a failure value.
 ///
 /// This function can be used to wrap calls to fallible functions ONLY when it
@@ -700,18 +717,10 @@
 ///
 

[PATCH] D62445: [test] Fix plugin tests

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D62445#1630518 , @NoQ wrote:

> I didn't make much progress so far, but i marked the test as `// UNSUPPORTED: 
> darwin` in rC368765 , so the buildbot is 
> now green starting with 
> http://green.lab.llvm.org/green/job/clang-stage2-cmake-RgSan/6519/.


I haven't had a chance to check this, but I suspect these plugin tests need 
LLVM_NO_DEAD_STRIP, since the failing build is RelWithDebInfo.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62445



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

[PATCH] D59754: [Sema] Add c++2a designated initializer warnings

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D59754#1651894 , @rsmith wrote:

> I'm taking this over to finish it off and submit.


Thanks Richard...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59754



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

[PATCH] D62445: [test] Fix plugin tests

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D62445#1613268 , @NoQ wrote:

> Ugh, there seems to be one more forgotten buildbot with plugins problems: 
> http://green.lab.llvm.org/green/job/clang-stage2-cmake-RgSan/6406/consoleText




> It got suddenly fixed in 
> http://green.lab.llvm.org/green/job/clang-stage2-cmake-RgSan/6409/ but then 
> immediately failed again later and it's still failing in a similar manner, 
> and nobody noticed for two months =/

The fix was accidental and fortuitous.

r362399 tried to set LLVM_ENABLE_PLUGINS_default to the value of 
LLVM_ENABLE_PIC, but had a typo that actually turn off LLVM_ENABLE_PLUGINS, so 
the test was marked unsupported in build #6409.  That typo was fixed in r362492 
which caused the next build, #6411 and all subsequent builds, to fail.

Will now look into cause/fix.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62445



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

[PATCH] D62445: [test] Fix plugin tests

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D62445#1613268 , @NoQ wrote:

> Ugh, there seems to be one more forgotten buildbot with plugins problems: 
> http://green.lab.llvm.org/green/job/clang-stage2-cmake-RgSan/6406/consoleText
>
>   FAIL: Clang :: Analysis/checker-plugins.c (467 of 14955)
>    TEST 'Clang :: Analysis/checker-plugins.c' FAILED 
> 
>   Script:
>   --
>   : 'RUN: at line 3';   
> /Users/buildslave/jenkins/workspace/clang-stage2-cmake-RgSan/clang-build/bin/clang
>  -cc1 -internal-isystem 
> /Users/buildslave/jenkins/workspace/clang-stage2-cmake-RgSan/clang-build/lib/clang/9.0.0/include
>  -nostdsysteminc -analyze -analyzer-constraints=range -verify 
> /Users/buildslave/jenkins/workspace/clang-stage2-cmake-RgSan/llvm/tools/clang/test/Analysis/checker-plugins.c
> -load 
> /Users/buildslave/jenkins/workspace/clang-stage2-cmake-RgSan/clang-build/./lib/SampleAnalyzerPlugin.dylib
> -analyzer-checker='example.MainCallChecker'




> - Exit Code: 1 ```
> 
>   The error message doesn't seem to be very expressive. I also can't 
> reproduce it locally.
> 
>   It got suddenly fixed in 
> http://green.lab.llvm.org/green/job/clang-stage2-cmake-RgSan/6409/ but then 
> immediately failed again later and it's still failing in a similar manner, 
> and nobody noticed for two months =/
> 
>   I'll keep looking into this but i'd love to hear if you have any immediate 
> thoughts on that :)

I don't see anything obvious, but could change the bot to pass "-vv" to lit 
instead of just "-v"?  That way the actual error will make to the log.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62445



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

[PATCH] D62105: [CommandLine] Remove OptionCategory and SubCommand caches from the Option class.

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 208736.
hintonda added a comment.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous.
Herald added a project: clang.

Make GeneralCategory a ManagedStatic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62105

Files:
  clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
  clang-tools-extra/clangd/indexer/IndexerMain.cpp
  clang/tools/clang-refactor/ClangRefactor.cpp
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp
  llvm/unittests/Support/CommandLineTest.cpp

Index: llvm/unittests/Support/CommandLineTest.cpp
===
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -95,16 +95,16 @@
   cl::Option *Retrieved = Map["test-option"];
   ASSERT_EQ(&TestOption, Retrieved) << "Retrieved wrong option.";
 
-  ASSERT_NE(Retrieved->Categories.end(),
-find_if(Retrieved->Categories,
+  ASSERT_NE(Retrieved->getCategories().end(),
+find_if(Retrieved->getCategories(),
 [&](const llvm::cl::OptionCategory *Cat) {
-  return Cat == &cl::GeneralCategory;
+  return Cat == &*cl::GeneralCategory;
 }))
   << "Incorrect default option category.";
 
   Retrieved->addCategory(TestCategory);
-  ASSERT_NE(Retrieved->Categories.end(),
-find_if(Retrieved->Categories,
+  ASSERT_NE(Retrieved->getCategories().end(),
+find_if(Retrieved->getCategories(),
 [&](const llvm::cl::OptionCategory *Cat) {
   return Cat == &TestCategory;
 }))
@@ -160,8 +160,8 @@
 TEST(CommandLineTest, UseOptionCategory) {
   StackOption TestOption2("test-option", cl::cat(TestCategory));
 
-  ASSERT_NE(TestOption2.Categories.end(),
-find_if(TestOption2.Categories,
+  ASSERT_NE(TestOption2.getCategories().end(),
+find_if(TestOption2.getCategories(),
  [&](const llvm::cl::OptionCategory *Cat) {
return Cat == &TestCategory;
  }))
@@ -170,42 +170,44 @@
 
 TEST(CommandLineTest, UseMultipleCategories) {
   StackOption TestOption2("test-option2", cl::cat(TestCategory),
-   cl::cat(cl::GeneralCategory),
-   cl::cat(cl::GeneralCategory));
+   cl::cat(*cl::GeneralCategory),
+   cl::cat(*cl::GeneralCategory));
+  auto TestOption2Categories = TestOption2.getCategories();
 
   // Make sure cl::GeneralCategory wasn't added twice.
-  ASSERT_EQ(TestOption2.Categories.size(), 2U);
+  ASSERT_EQ(TestOption2Categories.size(), 2U);
 
-  ASSERT_NE(TestOption2.Categories.end(),
-find_if(TestOption2.Categories,
+  ASSERT_NE(TestOption2Categories.end(),
+find_if(TestOption2Categories,
  [&](const llvm::cl::OptionCategory *Cat) {
return Cat == &TestCategory;
  }))
   << "Failed to assign Option Category.";
-  ASSERT_NE(TestOption2.Categories.end(),
-find_if(TestOption2.Categories,
+  ASSERT_NE(TestOption2Categories.end(),
+find_if(TestOption2Categories,
  [&](const llvm::cl::OptionCategory *Cat) {
-   return Cat == &cl::GeneralCategory;
+   return Cat == &*cl::GeneralCategory;
  }))
   << "Failed to assign General Category.";
 
   cl::OptionCategory AnotherCategory("Additional test Options", "Description");
   StackOption TestOption("test-option", cl::cat(TestCategory),
   cl::cat(AnotherCategory));
-  ASSERT_EQ(TestOption.Categories.end(),
-find_if(TestOption.Categories,
+  auto TestOptionCategories = TestOption.getCategories();
+  ASSERT_EQ(TestOptionCategories.end(),
+find_if(TestOptionCategories,
  [&](const llvm::cl::OptionCategory *Cat) {
-   return Cat == &cl::GeneralCategory;
+   return Cat == &*cl::GeneralCategory;
  }))
   << "Failed to remove General Category.";
-  ASSERT_NE(TestOption.Categories.end(),
-find_if(TestOption.Categories,
+  ASSERT_NE(TestOptionCategories.end(),
+find_if(TestOptionCategories,
  [&](const llvm::cl::OptionCategory *Cat) {
return Cat == &TestCategory;
  }))
   << "Failed to assign Option Category.";
-  ASSERT_NE(TestOption.Categories.end(),
-find_if(TestOption.Categories,
+  ASSERT_NE(TestOptionCategories.end(),
+find_if(TestOptionCategories,
  [&](const llvm::cl::OptionCat

[PATCH] D62926: [analyzer] ReturnVisitor: Bypass everything to see inlined calls

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

This test fails to compile on Windows 64 bit builds.  Please see 
http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/77




Comment at: cfe/trunk/test/Analysis/inlining/placement-new-fp-suppression.cpp:16
+
+typedef unsigned long uintptr_t;
+

I don't believe this is really ever portable, but definitely not on 64 bit 
Windows where a long is 32 bits.

Can't you just `#include ` instead?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62926



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

[PATCH] D62839: [clangd] Index API and implementations for relations

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

This seems to have broken gcc 5.4 builds -- for example see 
http://lab.llvm.org:8011/builders/clang-cmake-armv7-lnt/builds/29/steps/build%20stage%201/logs/stdio.

I believe the following patch should fix the problem:

  diff --git a/clang-tools-extra/clangd/index/FileIndex.cpp 
b/clang-tools-extra/clangd/index/FileIndex.cpp
  index a90c51b0990..802e4c4a396 100644
  --- a/clang-tools-extra/clangd/index/FileIndex.cpp
  +++ b/clang-tools-extra/clangd/index/FileIndex.cpp
  @@ -72,7 +72,7 @@ static SlabTuple indexSymbols(ASTContext &AST, 
std::shared_ptr PP,
  "  relations slab: {7} relations, {8} bytes",
  FileName, IsIndexMainAST, Syms.size(), Syms.bytes(), Refs.size(),
  Refs.numRefs(), Refs.bytes(), Relations.size(), Relations.bytes());
  -  return {std::move(Syms), std::move(Refs), std::move(Relations)};
  +  return std::make_tuple(std::move(Syms), std::move(Refs), 
std::move(Relations));
   }
  
   SlabTuple indexMainDecls(ParsedAST &AST) {


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62839



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

[PATCH] D59754: [Sema] Add c++2a designated initializer warnings

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D59754#1543949 , @sokolovd wrote:

> This looks like exactly what we need for my project. We're using Clang and 
> Designated Initializers but would like to make sure that we use those in 
> C++20 compatible manner. Is this blocked on something? Any way I can help?


The basic logic is here modulo @rsmith's comments.  I just haven't had time to 
address them, and unfortunately, it might be another few weeks before I can get 
back to it.

If you'd like to work on it and finish it up, that would be great.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59754



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

[PATCH] D61697: [lit] Disable test on darwin when building shared libs.

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D61697#1543443 , @thakis wrote:

> I reverted this in r363379 to unbreak check-lld on mac. I think the
>
>   if config.enable_shared:
>  features.add("enable_shared")
>   
>
> bit belongs in clang/test/lit.cfg.py, not in 
> llvm/utils/lit/lit/llvm/config.py.


Sorry the breakage, and thanks for the suggestion.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61697



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

[PATCH] D62873: Avoid building analyzer plugins if CLANG_ENABLE_STATIC_ANALYZER is OFF

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D62873#1530197 , @Szelethus wrote:

> I seem to have missed out on previous commits that moved analyzer plugins 
> around -- would you mind adding `[analyzer]` to the revision names that 
> affect the Static Analyzer? Many of us are automatically subscribed to such 
> patches.
>
> In any case, LGTM!


Sorry about that.  I was mainly trying to fix broken llvm plugins that were no 
longer available with the previous changes to LLVM_ENABLE_PLUGIN.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62873



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

[PATCH] D62873: Avoid building analyzer plugins if CLANG_ENABLE_STATIC_ANALYZER is OFF

[Don Hinton via Phabricator via cfe-commits]

hintonda added inline comments.



Comment at: lib/Analysis/plugins/CMakeLists.txt:1
-if(LLVM_ENABLE_PLUGINS)
+if(LLVM_ENABLE_PLUGINS AND CLANG_ENABLE_STATIC_ANALYZER)
   add_subdirectory(SampleAnalyzer)

Szelethus wrote:
> Is this file a thing? `lib/Analysis/plugins/CMakeLists.txt`? I can't seem to 
> find it anywhere.
commit a33eaad00cca99ca0b5d2b0cc6dec33be6d7ee7f (origin/master, origin/HEAD, 
master)
Author: Don Hinton 
Date:   Tue Jun 4 22:07:40 2019 +

[Analysis] Only build Analysis plugins when CLANG_ENABLE_STATIC_ANALYZER is 
enabled.

Fixes bug introduced in r362328.

Thanks to Nathan Chancellor for reporting this!

llvm-svn: 362555

$ git diff  06c801e153347d24ec7ce93f6ffbbc58b64a89ba 
a33eaad00cca99ca0b5d2b0cc6dec33be6d7ee7f
diff --git a/clang/lib/Analysis/plugins/CMakeLists.txt 
b/clang/lib/Analysis/plugins/CMakeLists.txt
index f7dbc936952..bd7314a871f 100644
--- a/clang/lib/Analysis/plugins/CMakeLists.txt
+++ b/clang/lib/Analysis/plugins/CMakeLists.txt
@@ -1,4 +1,4 @@
-if(LLVM_ENABLE_PLUGINS)
+if(CLANG_ENABLE_STATIC_ANALYZER AND LLVM_ENABLE_PLUGINS)
   add_subdirectory(SampleAnalyzer)
   add_subdirectory(CheckerDependencyHandling)
   add_subdirectory(CheckerOptionHandling)


Repository:
  rC Clang

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

https://reviews.llvm.org/D62873



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

[PATCH] D62873: Avoid building analyzer plugins if CLANG_ENABLE_STATIC_ANALYZER is OFF

[Don Hinton via Phabricator via cfe-commits]

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

LGTM...




Comment at: test/CMakeLists.txt:122
 
-if (CLANG_ENABLE_STATIC_ANALYZER)
-  if (LLVM_ENABLE_PLUGINS)
-list(APPEND CLANG_TEST_DEPS
-  SampleAnalyzerPlugin
-  CheckerDependencyHandlingAnalyzerPlugin
-  CheckerOptionHandlingAnalyzerPlugin
-  )
-  endif()
+if(LLVM_ENABLE_PLUGINS AND CLANG_ENABLE_STATIC_ANALYZER)
+  list(APPEND CLANG_TEST_DEPS

I didn't fix this one, but makes sense. 


Repository:
  rC Clang

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

https://reviews.llvm.org/D62873



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

[PATCH] D62873: Avoid building analyzer plugins if CLANG_ENABLE_STATIC_ANALYZER is OFF

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D62873#1529800 , @nathanchance 
wrote:

> Thanks for the quick fix, looks good to me!


Ah, sorry, just saw this.  I just committed r362555 that addresses this.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62873



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

[PATCH] D62445: [test] Fix plugin tests

[Don Hinton via Phabricator via cfe-commits]

hintonda marked an inline comment as done.
hintonda added inline comments.



Comment at: cfe/trunk/lib/Analysis/plugins/CMakeLists.txt:1
+if(LLVM_ENABLE_PLUGINS)
+  add_subdirectory(SampleAnalyzer)

hintonda wrote:
> nathanchance wrote:
> > I think this should have a dependency on `CLANG_ENABLE_STATIC_ANALYZER` 
> > like in `clang/test/CMakeLists.txt`, otherwise my build (which disables 
> > `CLANG_ENABLE_STATIC_ANALYZER`) fails because these plugins are being added 
> > to the build.
> > 
> > ```
> > [2058/2605] Linking CXX shared module 
> > lib/CheckerOptionHandlingAnalyzerPlugin.so
> > FAILED: lib/CheckerOptionHandlingAnalyzerPlugin.so 
> > : && /usr/bin/g++ -fPIC -O2 -march=native -mtune=native -fPIC 
> > -fvisibility-inlines-hidden -Werror=date-time -std=c++11 -w 
> > -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common 
> > -Woverloaded-virtual -fno-strict-aliasing -O3 -DNDEBUG  -Wl,-z,nodelete 
> > -fuse-ld=gold   -Wl,-O3 -Wl,--gc-sections  
> > -Wl,--version-script,"/tc-build2/build/llvm/stage1/tools/clang/lib/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandlingAnalyzerPlugin.exports"
> >  -shared  -o lib/CheckerOptionHandlingAnalyzerPlugin.so 
> > tools/clang/lib/Analysis/plugins/CheckerOptionHandling/CMakeFiles/CheckerOptionHandlingAnalyzerPlugin.dir/CheckerOptionHandling.cpp.o
> >   -Wl,-rpath,"\$ORIGIN/../lib" lib/libLLVMSupport.a lib/libclangAnalysis.a 
> > lib/libclangAST.a -lclangStaticAnalyzerCore -lclangStaticAnalyzerFrontend 
> > lib/libLLVMSupport.a lib/libclangASTMatchers.a lib/libclangAST.a 
> > lib/libclangLex.a lib/libclangBasic.a lib/libLLVMCore.a 
> > lib/libLLVMRemarks.a lib/libLLVMMC.a lib/libLLVMBinaryFormat.a 
> > lib/libLLVMDebugInfoCodeView.a lib/libLLVMDebugInfoMSF.a 
> > lib/libLLVMSupport.a -lz -lrt -ldl -lpthread -lm lib/libLLVMDemangle.a && :
> > /usr/bin/ld.gold: error: cannot find -lclangStaticAnalyzerCore
> > /usr/bin/ld.gold: error: cannot find -lclangStaticAnalyzerFrontend
> > collect2: error: ld returned 1 exit status
> > [2059/2605] Building CXX object 
> > tools/clang/tools/clang-refactor/CMakeFiles/clang-refactor.dir/TestSupport.cpp.o
> > [2060/2605] Linking CXX shared module 
> > lib/CheckerDependencyHandlingAnalyzerPlugin.so
> > FAILED: lib/CheckerDependencyHandlingAnalyzerPlugin.so 
> > : && /usr/bin/g++ -fPIC -O2 -march=native -mtune=native -fPIC 
> > -fvisibility-inlines-hidden -Werror=date-time -std=c++11 -w 
> > -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common 
> > -Woverloaded-virtual -fno-strict-aliasing -O3 -DNDEBUG  -Wl,-z,nodelete 
> > -fuse-ld=gold   -Wl,-O3 -Wl,--gc-sections  
> > -Wl,--version-script,"/tc-build2/build/llvm/stage1/tools/clang/lib/Analysis/plugins/CheckerDependencyHandling/CheckerDependencyHandlingAnalyzerPlugin.exports"
> >  -shared  -o lib/CheckerDependencyHandlingAnalyzerPlugin.so 
> > tools/clang/lib/Analysis/plugins/CheckerDependencyHandling/CMakeFiles/CheckerDependencyHandlingAnalyzerPlugin.dir/CheckerDependencyHandling.cpp.o
> >   -Wl,-rpath,"\$ORIGIN/../lib" lib/libLLVMSupport.a lib/libclangAnalysis.a 
> > lib/libclangAST.a -lclangStaticAnalyzerCore -lclangStaticAnalyzerFrontend 
> > lib/libLLVMSupport.a lib/libclangASTMatchers.a lib/libclangAST.a 
> > lib/libclangLex.a lib/libclangBasic.a lib/libLLVMCore.a 
> > lib/libLLVMRemarks.a lib/libLLVMMC.a lib/libLLVMBinaryFormat.a 
> > lib/libLLVMDebugInfoCodeView.a lib/libLLVMDebugInfoMSF.a 
> > lib/libLLVMSupport.a -lz -lrt -ldl -lpthread -lm lib/libLLVMDemangle.a && :
> > /usr/bin/ld.gold: error: cannot find -lclangStaticAnalyzerCore
> > /usr/bin/ld.gold: error: cannot find -lclangStaticAnalyzerFrontend
> > collect2: error: ld returned 1 exit status
> > [2061/2605] Building CXX object 
> > tools/clang/lib/Tooling/CMakeFiles/obj.clangTooling.dir/CompilationDatabase.cpp.o
> > [2062/2605] Linking CXX shared module lib/SampleAnalyzerPlugin.so
> > FAILED: lib/SampleAnalyzerPlugin.so 
> > : && /usr/bin/g++ -fPIC -O2 -march=native -mtune=native -fPIC 
> > -fvisibility-inlines-hidden -Werror=date-time -std=c++11 -w 
> > -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common 
> > -Woverloaded-virtual -fno-strict-aliasing -O3 -DNDEBUG  -Wl,-z,nodelete 
> > -fuse-ld=gold   -Wl,-O3 -Wl,--gc-sections  
> > -Wl,--version-script,"/tc-build2/build/llvm/stage1/tools/clang/lib/Analysis/plugins/SampleAnalyzer/SampleAnalyzerPlugin.exports"
> >  -shared  -o lib/SampleAnalyzerPlugin.so 
> > tools/clang/lib/Analysis/plugins/SampleAnalyzer/CMakeFiles/SampleAnalyzerPlugin.dir/MainCallChecker.cpp.o
> >   -Wl,-rpath,"\$ORIGIN/../lib" lib/libLLVMSupport.a lib/libclangAnalysis.a 
> > lib/libclangAST.a -lclangStaticAnalyzerCore -lclangStaticAnalyzerFrontend 
> > lib/libLLVMSupport.a lib/libclangASTMatchers.a lib/libclangAST.a 
> > lib/libclangLex.a lib/libclangBasic.a lib/libLLVMCore.a 
> > lib/libLLVMRemarks.a lib/libLLVMMC.a lib/libLLVMBinaryFormat.a 
> > lib/libLLVMDebugInfoCodeView.a lib/libLLVMDebugInfoMSF.a 
> > li

[PATCH] D62445: [test] Fix plugin tests

[Don Hinton via Phabricator via cfe-commits]

hintonda marked an inline comment as done.
hintonda added inline comments.



Comment at: cfe/trunk/lib/Analysis/plugins/CMakeLists.txt:1
+if(LLVM_ENABLE_PLUGINS)
+  add_subdirectory(SampleAnalyzer)

nathanchance wrote:
> I think this should have a dependency on `CLANG_ENABLE_STATIC_ANALYZER` like 
> in `clang/test/CMakeLists.txt`, otherwise my build (which disables 
> `CLANG_ENABLE_STATIC_ANALYZER`) fails because these plugins are being added 
> to the build.
> 
> ```
> [2058/2605] Linking CXX shared module 
> lib/CheckerOptionHandlingAnalyzerPlugin.so
> FAILED: lib/CheckerOptionHandlingAnalyzerPlugin.so 
> : && /usr/bin/g++ -fPIC -O2 -march=native -mtune=native -fPIC 
> -fvisibility-inlines-hidden -Werror=date-time -std=c++11 -w 
> -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common 
> -Woverloaded-virtual -fno-strict-aliasing -O3 -DNDEBUG  -Wl,-z,nodelete 
> -fuse-ld=gold   -Wl,-O3 -Wl,--gc-sections  
> -Wl,--version-script,"/tc-build2/build/llvm/stage1/tools/clang/lib/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandlingAnalyzerPlugin.exports"
>  -shared  -o lib/CheckerOptionHandlingAnalyzerPlugin.so 
> tools/clang/lib/Analysis/plugins/CheckerOptionHandling/CMakeFiles/CheckerOptionHandlingAnalyzerPlugin.dir/CheckerOptionHandling.cpp.o
>   -Wl,-rpath,"\$ORIGIN/../lib" lib/libLLVMSupport.a lib/libclangAnalysis.a 
> lib/libclangAST.a -lclangStaticAnalyzerCore -lclangStaticAnalyzerFrontend 
> lib/libLLVMSupport.a lib/libclangASTMatchers.a lib/libclangAST.a 
> lib/libclangLex.a lib/libclangBasic.a lib/libLLVMCore.a lib/libLLVMRemarks.a 
> lib/libLLVMMC.a lib/libLLVMBinaryFormat.a lib/libLLVMDebugInfoCodeView.a 
> lib/libLLVMDebugInfoMSF.a lib/libLLVMSupport.a -lz -lrt -ldl -lpthread -lm 
> lib/libLLVMDemangle.a && :
> /usr/bin/ld.gold: error: cannot find -lclangStaticAnalyzerCore
> /usr/bin/ld.gold: error: cannot find -lclangStaticAnalyzerFrontend
> collect2: error: ld returned 1 exit status
> [2059/2605] Building CXX object 
> tools/clang/tools/clang-refactor/CMakeFiles/clang-refactor.dir/TestSupport.cpp.o
> [2060/2605] Linking CXX shared module 
> lib/CheckerDependencyHandlingAnalyzerPlugin.so
> FAILED: lib/CheckerDependencyHandlingAnalyzerPlugin.so 
> : && /usr/bin/g++ -fPIC -O2 -march=native -mtune=native -fPIC 
> -fvisibility-inlines-hidden -Werror=date-time -std=c++11 -w 
> -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common 
> -Woverloaded-virtual -fno-strict-aliasing -O3 -DNDEBUG  -Wl,-z,nodelete 
> -fuse-ld=gold   -Wl,-O3 -Wl,--gc-sections  
> -Wl,--version-script,"/tc-build2/build/llvm/stage1/tools/clang/lib/Analysis/plugins/CheckerDependencyHandling/CheckerDependencyHandlingAnalyzerPlugin.exports"
>  -shared  -o lib/CheckerDependencyHandlingAnalyzerPlugin.so 
> tools/clang/lib/Analysis/plugins/CheckerDependencyHandling/CMakeFiles/CheckerDependencyHandlingAnalyzerPlugin.dir/CheckerDependencyHandling.cpp.o
>   -Wl,-rpath,"\$ORIGIN/../lib" lib/libLLVMSupport.a lib/libclangAnalysis.a 
> lib/libclangAST.a -lclangStaticAnalyzerCore -lclangStaticAnalyzerFrontend 
> lib/libLLVMSupport.a lib/libclangASTMatchers.a lib/libclangAST.a 
> lib/libclangLex.a lib/libclangBasic.a lib/libLLVMCore.a lib/libLLVMRemarks.a 
> lib/libLLVMMC.a lib/libLLVMBinaryFormat.a lib/libLLVMDebugInfoCodeView.a 
> lib/libLLVMDebugInfoMSF.a lib/libLLVMSupport.a -lz -lrt -ldl -lpthread -lm 
> lib/libLLVMDemangle.a && :
> /usr/bin/ld.gold: error: cannot find -lclangStaticAnalyzerCore
> /usr/bin/ld.gold: error: cannot find -lclangStaticAnalyzerFrontend
> collect2: error: ld returned 1 exit status
> [2061/2605] Building CXX object 
> tools/clang/lib/Tooling/CMakeFiles/obj.clangTooling.dir/CompilationDatabase.cpp.o
> [2062/2605] Linking CXX shared module lib/SampleAnalyzerPlugin.so
> FAILED: lib/SampleAnalyzerPlugin.so 
> : && /usr/bin/g++ -fPIC -O2 -march=native -mtune=native -fPIC 
> -fvisibility-inlines-hidden -Werror=date-time -std=c++11 -w 
> -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common 
> -Woverloaded-virtual -fno-strict-aliasing -O3 -DNDEBUG  -Wl,-z,nodelete 
> -fuse-ld=gold   -Wl,-O3 -Wl,--gc-sections  
> -Wl,--version-script,"/tc-build2/build/llvm/stage1/tools/clang/lib/Analysis/plugins/SampleAnalyzer/SampleAnalyzerPlugin.exports"
>  -shared  -o lib/SampleAnalyzerPlugin.so 
> tools/clang/lib/Analysis/plugins/SampleAnalyzer/CMakeFiles/SampleAnalyzerPlugin.dir/MainCallChecker.cpp.o
>   -Wl,-rpath,"\$ORIGIN/../lib" lib/libLLVMSupport.a lib/libclangAnalysis.a 
> lib/libclangAST.a -lclangStaticAnalyzerCore -lclangStaticAnalyzerFrontend 
> lib/libLLVMSupport.a lib/libclangASTMatchers.a lib/libclangAST.a 
> lib/libclangLex.a lib/libclangBasic.a lib/libLLVMCore.a lib/libLLVMRemarks.a 
> lib/libLLVMMC.a lib/libLLVMBinaryFormat.a lib/libLLVMDebugInfoCodeView.a 
> lib/libLLVMDebugInfoMSF.a lib/libLLVMSupport.a -lz -lrt -ldl -lpthread -lm 
> lib/libLLVMDemangle.a && :
> /usr/bin/ld.gold: error: cannot find -lclangStaticAnalyzerCore
> /usr/bin/ld.gol

[PATCH] D62445: [test] Fix plugin tests

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D62445#1527368 , @davezarzycki 
wrote:

> This broke non-PIC builds. Fixed: r362399


Sorry for the breakage, and thanks for the fix!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62445



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

[PATCH] D62445: [test] Fix plugin tests

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 202573.
hintonda added a comment.

- Fix dependencies so required plugins get built before tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62445

Files:
  clang/lib/Analysis/CMakeLists.txt
  clang/lib/Analysis/plugins/CMakeLists.txt
  clang/lib/Analysis/plugins/CheckerDependencyHandling/CMakeLists.txt
  
clang/lib/Analysis/plugins/CheckerDependencyHandling/CheckerDependencyHandling.cpp
  
clang/lib/Analysis/plugins/CheckerDependencyHandling/CheckerDependencyHandlingAnalyzerPlugin.exports
  clang/lib/Analysis/plugins/CheckerOptionHandling/CMakeLists.txt
  clang/lib/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandling.cpp
  
clang/lib/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandlingAnalyzerPlugin.exports
  clang/lib/Analysis/plugins/SampleAnalyzer/CMakeLists.txt
  clang/lib/Analysis/plugins/SampleAnalyzer/MainCallChecker.cpp
  clang/lib/Analysis/plugins/SampleAnalyzer/SampleAnalyzerPlugin.exports
  clang/test/Analysis/lit.local.cfg
  clang/test/Analysis/plugins/CMakeLists.txt
  clang/test/Analysis/plugins/CheckerDependencyHandling/CMakeLists.txt
  
clang/test/Analysis/plugins/CheckerDependencyHandling/CheckerDependencyHandling.cpp
  
clang/test/Analysis/plugins/CheckerDependencyHandling/CheckerDependencyHandlingAnalyzerPlugin.exports
  clang/test/Analysis/plugins/CheckerOptionHandling/CMakeLists.txt
  clang/test/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandling.cpp
  
clang/test/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandlingAnalyzerPlugin.exports
  clang/test/Analysis/plugins/SampleAnalyzer/CMakeLists.txt
  clang/test/Analysis/plugins/SampleAnalyzer/MainCallChecker.cpp
  clang/test/Analysis/plugins/SampleAnalyzer/SampleAnalyzerPlugin.exports
  clang/test/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/modules/HandleLLVMOptions.cmake

Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -912,14 +912,6 @@
   message(FATAL_ERROR "LLVM_LINK_LLVM_DYLIB not compatible with LLVM_EXPORT_SYMBOLS_FOR_PLUGINS")
 endif()
 
-# Plugin support
-# FIXME: Make this configurable.
-if(BUILD_SHARED_LIBS OR LLVM_BUILD_LLVM_DYLIB)
-  set(LLVM_ENABLE_PLUGINS ON)
-else()
-  set(LLVM_ENABLE_PLUGINS OFF)
-endif()
-
 # By default we should enable LLVM_ENABLE_IDE only for multi-configuration
 # generators. This option disables optional build system features that make IDEs
 # less usable.
Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -672,6 +672,17 @@
 message(STATUS "LLVM host triple: ${LLVM_HOST_TRIPLE}")
 message(STATUS "LLVM default target triple: ${LLVM_DEFAULT_TARGET_TRIPLE}")
 
+if(WIN32 OR CYGWIN)
+  if(BUILD_SHARED_LIBS OR LLVM_BUILD_LLVM_DYLIB)
+set(LLVM_ENABLE_PLUGINS_default ON)
+  else()
+set(LLVM_ENABLE_PLUGINS_default OFF)
+  endif()
+else()
+  set(LLVM_ENABLE_PLUGINS_default ON)
+endif()
+option(LLVM_ENABLE_PLUGINS "Enable plugin support" ${LLVM_ENABLE_PLUGINS_default})
+
 include(HandleLLVMOptions)
 
 # Verify that we can find a Python 2 interpreter.  Python 3 is unsupported.
Index: clang/test/CMakeLists.txt
===
--- clang/test/CMakeLists.txt
+++ clang/test/CMakeLists.txt
@@ -83,12 +83,6 @@
 )
 endif ()
 
-if (CLANG_ENABLE_STATIC_ANALYZER AND CLANG_BUILD_EXAMPLES)
-  list(APPEND CLANG_TEST_DEPS
-SampleAnalyzerPlugin
-)
-endif ()
-
 set(CLANG_TEST_PARAMS
   clang_site_config=${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg
   USE_Z3_SOLVER=0
@@ -126,27 +120,13 @@
 endif()
 
 if (CLANG_ENABLE_STATIC_ANALYZER)
-  add_subdirectory(Analysis/plugins)
-  list(APPEND CLANG_TEST_DEPS clang-analyzer-plugin)
-
-  # check-all would launch those tests via check-clang.
-  set(EXCLUDE_FROM_ALL ON)
-
-  add_lit_testsuite(check-clang-analyzer "Running the Clang analyzer tests"
-${CMAKE_CURRENT_BINARY_DIR}/Analysis
-PARAMS ${ANALYZER_TEST_PARAMS}
-DEPENDS ${CLANG_TEST_DEPS})
-  set_target_properties(check-clang-analyzer PROPERTIES FOLDER "Clang tests")
-
-  if (LLVM_WITH_Z3)
-add_lit_testsuite(check-clang-analyzer-z3 "Running the Clang analyzer tests, using Z3 as a solver"
-  ${CMAKE_CURRENT_BINARY_DIR}/Analysis
-  PARAMS ${ANALYZER_TEST_PARAMS_Z3}
-  DEPENDS ${CLANG_TEST_DEPS})
-set_target_properties(check-clang-analyzer-z3 PROPERTIES FOLDER "Clang tests")
+  if (LLVM_ENABLE_PLUGINS)
+set(CLANG_TEST_DEPS
+  SampleAnalyzerPlugin
+  CheckerDependencyHandlingAnalyzerPlugin
+  CheckerOptionHandlingAnalyzerPlugin
+  )
   endif()
-
-  set(EXCLUDE_FROM_ALL OFF)
 endif()
 
 add_custom_target(clang-test-depends DEPENDS ${CLANG_TEST_DEPS})
Index: clang/test/Analysis/plugins/SampleAnalyzer/C

[PATCH] D62445: [test] Fix plugin tests

[Don Hinton via Phabricator via cfe-commits]

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

This patch broke some of the bots, so reopening to track the fix.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62445



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

[PATCH] D61697: [lit] Disable test on darwin when building shared libs.

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

ping...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61697



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

[PATCH] D62445: [test] Fix plugin tests

[Don Hinton via Phabricator via cfe-commits]

hintonda marked an inline comment as done.
hintonda added inline comments.



Comment at: clang/test/CMakeLists.txt:141-147
-  if (LLVM_WITH_Z3)
-add_lit_testsuite(check-clang-analyzer-z3 "Running the Clang analyzer 
tests, using Z3 as a solver"
-  ${CMAKE_CURRENT_BINARY_DIR}/Analysis
-  PARAMS ${ANALYZER_TEST_PARAMS_Z3}
-  DEPENDS ${CLANG_TEST_DEPS})
-set_target_properties(check-clang-analyzer-z3 PROPERTIES FOLDER "Clang 
tests")
   endif()

hintonda wrote:
> NoQ wrote:
> > As far as i remember, this chunk of code is responsible for running the 
> > whole analyzer test suite with different parameters (with Z3 as a 
> > constraint manager instead of the ad-hoc range constraint manager), rather 
> > than running a smaller chunk of the suite.
> > 
> > @mikhail.ramalho, @ddcc - i think i should leave it up to you to decide if 
> > you want to keep this working. Right now these extra tests aren't run under 
> > any buildbot, and the facility that they're testing is probably never going 
> > to be used by actual users (as opposed to the z3 refutation). But i 
> > wouldn't love losing this facility because it's a great way of evaluating 
> > the static analyzer, i.e. figuring out how good *could* it have been with a 
> > good constraint solver, so that we had something to look forward to.
> Ah, that was unintentional.  I didn't intend to touch Z3.
> 
> Let me see if I can put that part back.
Actually, on second thought, these get picked up automatically, so they 
shouldn't be added like this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62445



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

[PATCH] D62445: [test] Fix plugin tests

[Don Hinton via Phabricator via cfe-commits]

hintonda marked an inline comment as done.
hintonda added inline comments.



Comment at: clang/test/CMakeLists.txt:141-147
-  if (LLVM_WITH_Z3)
-add_lit_testsuite(check-clang-analyzer-z3 "Running the Clang analyzer 
tests, using Z3 as a solver"
-  ${CMAKE_CURRENT_BINARY_DIR}/Analysis
-  PARAMS ${ANALYZER_TEST_PARAMS_Z3}
-  DEPENDS ${CLANG_TEST_DEPS})
-set_target_properties(check-clang-analyzer-z3 PROPERTIES FOLDER "Clang 
tests")
   endif()

NoQ wrote:
> As far as i remember, this chunk of code is responsible for running the whole 
> analyzer test suite with different parameters (with Z3 as a constraint 
> manager instead of the ad-hoc range constraint manager), rather than running 
> a smaller chunk of the suite.
> 
> @mikhail.ramalho, @ddcc - i think i should leave it up to you to decide if 
> you want to keep this working. Right now these extra tests aren't run under 
> any buildbot, and the facility that they're testing is probably never going 
> to be used by actual users (as opposed to the z3 refutation). But i wouldn't 
> love losing this facility because it's a great way of evaluating the static 
> analyzer, i.e. figuring out how good *could* it have been with a good 
> constraint solver, so that we had something to look forward to.
Ah, that was unintentional.  I didn't intend to touch Z3.

Let me see if I can put that part back.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62445



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

[PATCH] D62445: [test] Fix plugin tests

[Don Hinton via Phabricator via cfe-commits]

hintonda created this revision.
hintonda added reviewers: beanz, phosek, george.karpenkov, NoQ, ahatanak, 
rjmccall.
Herald added subscribers: llvm-commits, dexonsmith, mgorny.
Herald added projects: clang, LLVM.
hintonda edited the summary of this revision.

The following changes were required to fix these tests:

1. Change LLVM_ENABLE_PLUGINS to an option and move it to llvm/CMakeLists.txt 
with an appropriate default -- which matches the original default behavior.

2. Move the plugins directory from clang/test/Analysis clang/lib/Analysis.  
It's not enough to add an exclude to the lit.local.cfg file because 
add_lit_testsuites recurses the tree and automatically adds the appropriate 
`check-` targets, which don't make sense for the plugins because they aren't 
tests and don't have no `RUN` statements.

  Here's a list of the `clang-check-anlysis*` targets with this change:

  $ ninja -t targets all| sed -n "s/.*\/\(check[^:]*\):.*/\1/p" | sort -u | 
grep clang-analysis
  check-clang-analysis
  check-clang-analysis-checkers
  check-clang-analysis-copypaste
  check-clang-analysis-diagnostics
  check-clang-analysis-engine
  check-clang-analysis-exploration_order
  check-clang-analysis-html_diagnostics
  check-clang-analysis-html_diagnostics-relevant_lines
  check-clang-analysis-inlining
  check-clang-analysis-objc
  check-clang-analysis-unified-sources
  check-clang-analysis-z3



3. Simplify the logic and only include the subdirectories under 
clang/lib/Analysis/plugins if LLVM_ENABLE_PLUGINS is set.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D62445

Files:
  clang/lib/Analysis/CMakeLists.txt
  clang/lib/Analysis/plugins/CMakeLists.txt
  clang/lib/Analysis/plugins/CheckerDependencyHandling/CMakeLists.txt
  
clang/lib/Analysis/plugins/CheckerDependencyHandling/CheckerDependencyHandling.cpp
  
clang/lib/Analysis/plugins/CheckerDependencyHandling/CheckerDependencyHandlingAnalyzerPlugin.exports
  clang/lib/Analysis/plugins/CheckerOptionHandling/CMakeLists.txt
  clang/lib/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandling.cpp
  
clang/lib/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandlingAnalyzerPlugin.exports
  clang/lib/Analysis/plugins/SampleAnalyzer/CMakeLists.txt
  clang/lib/Analysis/plugins/SampleAnalyzer/MainCallChecker.cpp
  clang/lib/Analysis/plugins/SampleAnalyzer/SampleAnalyzerPlugin.exports
  clang/test/Analysis/lit.local.cfg
  clang/test/Analysis/plugins/CMakeLists.txt
  clang/test/Analysis/plugins/CheckerDependencyHandling/CMakeLists.txt
  
clang/test/Analysis/plugins/CheckerDependencyHandling/CheckerDependencyHandling.cpp
  
clang/test/Analysis/plugins/CheckerDependencyHandling/CheckerDependencyHandlingAnalyzerPlugin.exports
  clang/test/Analysis/plugins/CheckerOptionHandling/CMakeLists.txt
  clang/test/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandling.cpp
  
clang/test/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandlingAnalyzerPlugin.exports
  clang/test/Analysis/plugins/SampleAnalyzer/CMakeLists.txt
  clang/test/Analysis/plugins/SampleAnalyzer/MainCallChecker.cpp
  clang/test/Analysis/plugins/SampleAnalyzer/SampleAnalyzerPlugin.exports
  clang/test/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/modules/HandleLLVMOptions.cmake

Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -912,14 +912,6 @@
   message(FATAL_ERROR "LLVM_LINK_LLVM_DYLIB not compatible with LLVM_EXPORT_SYMBOLS_FOR_PLUGINS")
 endif()
 
-# Plugin support
-# FIXME: Make this configurable.
-if(BUILD_SHARED_LIBS OR LLVM_BUILD_LLVM_DYLIB)
-  set(LLVM_ENABLE_PLUGINS ON)
-else()
-  set(LLVM_ENABLE_PLUGINS OFF)
-endif()
-
 # By default we should enable LLVM_ENABLE_IDE only for multi-configuration
 # generators. This option disables optional build system features that make IDEs
 # less usable.
Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -669,6 +669,17 @@
 message(STATUS "LLVM host triple: ${LLVM_HOST_TRIPLE}")
 message(STATUS "LLVM default target triple: ${LLVM_DEFAULT_TARGET_TRIPLE}")
 
+if(WIN32 OR CYGWIN)
+  if(BUILD_SHARED_LIBS OR LLVM_BUILD_LLVM_DYLIB)
+set(LLVM_ENABLE_PLUGINS_default ON)
+  else()
+set(LLVM_ENABLE_PLUGINS_default OFF)
+  endif()
+else()
+  set(LLVM_ENABLE_PLUGINS_default ON)
+endif()
+option(LLVM_ENABLE_PLUGINS "Enable plugin support" ${LLVM_ENABLE_PLUGINS_default})
+
 include(HandleLLVMOptions)
 
 # Verify that we can find a Python 2 interpreter.  Python 3 is unsupported.
Index: clang/test/CMakeLists.txt
===
--- clang/test/CMakeLists.txt
+++ clang/test/CMakeLists.txt
@@ -126,27 +126,13 @@
 endif()
 
 if (CLANG_ENABLE_STATIC_ANALYZER)
-  add_subdirectory(Analysis/plugins)
-  list(APPEND CLANG_TEST_DEPS clang-analyzer

[PATCH] D62343: [cmake] Remove old unused version of FindZ3.cmake from clang [NFC]

[Don Hinton via Phabricator via cfe-commits]

hintonda created this revision.
hintonda added reviewers: mikhail.ramalho, beanz.
Herald added a subscriber: mgorny.
Herald added a project: clang.

This file was moved to llvm in D54978 , 
r356929, but the old
file was never removed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D62343

Files:
  clang/cmake/modules/FindZ3.cmake




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

[PATCH] D62174: [Analysis] Link library dependencies to Analysis plugins

[Don Hinton via Phabricator via cfe-commits]

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

LGTM.  Build and check-llvm were both clean on my Mac for static build.  Thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D62174



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

[PATCH] D62050: [Analysis] Only run plugins tests if plugins are actually enabled

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D62050#1509400 , @vitalybuka wrote:

> In D62050#1509384 , @vitalybuka 
> wrote:
>
> > This also breaks "ninja check-cfi-and-supported" on clean build (run cmake 
> > in empty directory)
>
>
> I've fixed this with DLLVM_BUILD_LLVM_DYLIB=ON but not sure if this is the 
> right approach long-term.


I have a patch that only reverts the changes to HandleLLVMOptions.cmake, D62154 
, and restores previous behavior.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62050



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

[PATCH] D61697: [lit] Disable test on darwin when building shared libs.

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

ping...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61697



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

[PATCH] D62157: Remove explicit header-filter in run_clang_tidy.py

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D62157#1509289 , @juliehockett 
wrote:

> @hintonda Aha so it is :)


I don't think it's at all controversial, so I'll commit D61747 
 tomorrow unless someone raises an issue.  
Thanks...


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

https://reviews.llvm.org/D62157



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

[PATCH] D61747: [clang-tidy] remove default header-filter for run-clang-tidy

[Don Hinton via Phabricator via cfe-commits]

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

LGTM.  If no further comments, I'll commit it for you tomorrow.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61747



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

[PATCH] D62157: Remove explicit header-filter in run_clang_tidy.py

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

Great idea.

Please see D61747 .


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

https://reviews.llvm.org/D62157



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

[PATCH] D62050: [Analysis] Only run plugins tests if plugins are actually enabled

[Don Hinton via Phabricator via cfe-commits]

hintonda added inline comments.



Comment at: llvm/trunk/cmake/modules/HandleLLVMOptions.cmake:930
-else()
+if(BUILD_SHARED_LIBS OR LLVM_BUILD_LLVM_DYLIB)
   set(LLVM_ENABLE_PLUGINS ON)
 endif()

hintonda wrote:
> This is a breaking patch.  What's the rational for unilaterally disabling 
> LLVM_ENABLE_PLUGINS.
> 
> ```
> It gives the following errors:
> -- PrintFunctionNames ignored -- Loadable modules not supported on this 
> platform.
> -- AnnotateFunctions ignored -- Loadable modules not supported on this 
> platform.
> -- SampleAnalyzerPlugin ignored -- Loadable modules not supported on this 
> platform.
> -- CheckerDependencyHandlingAnalyzerPlugin ignored -- Loadable modules not 
> supported on this platform.
> -- CheckerOptionHandlingAnalyzerPlugin ignored -- Loadable modules not 
> supported on this platform.
> -- BugpointPasses ignored -- Loadable modules not supported on this platform.
> -- TestPlugin ignored -- Loadable modules not supported on this platform.
> ```
Btw, the change essentially turns off testing of plugins, and bugpoint, etc., 
in all the buildbots, but since it isn't reported as an error, they don't 
report that these tests are no longer run.,


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62050



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

[PATCH] D62050: [Analysis] Only run plugins tests if plugins are actually enabled

[Don Hinton via Phabricator via cfe-commits]

hintonda added inline comments.



Comment at: llvm/trunk/cmake/modules/HandleLLVMOptions.cmake:930
-else()
+if(BUILD_SHARED_LIBS OR LLVM_BUILD_LLVM_DYLIB)
   set(LLVM_ENABLE_PLUGINS ON)
 endif()

This is a breaking patch.  What's the rational for unilaterally disabling 
LLVM_ENABLE_PLUGINS.

```
It gives the following errors:
-- PrintFunctionNames ignored -- Loadable modules not supported on this 
platform.
-- AnnotateFunctions ignored -- Loadable modules not supported on this platform.
-- SampleAnalyzerPlugin ignored -- Loadable modules not supported on this 
platform.
-- CheckerDependencyHandlingAnalyzerPlugin ignored -- Loadable modules not 
supported on this platform.
-- CheckerOptionHandlingAnalyzerPlugin ignored -- Loadable modules not 
supported on this platform.
-- BugpointPasses ignored -- Loadable modules not supported on this platform.
-- TestPlugin ignored -- Loadable modules not supported on this platform.
```


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62050



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

[PATCH] D62138: [Docs] Increase Doxygen cache size

[Don Hinton via Phabricator via cfe-commits]

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

Thanks, LGTM!

I'll give it a day for further comments, and barring any, commit it for you 
tomorrow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62138



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

[PATCH] D61747: [clang-tidy] remove default header-filter for run-clang-tidy

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

After thinking about it, I agree with this change -- there's no way to provide 
an appropriate default at this level.

So, LGTM, but wait for @alexfh's okay.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61747



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

[PATCH] D61756: Add a __FILE_NAME__ macro.

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D61756#1504109 , @kristina wrote:

> Reverted in rL360842  as Windows bots were 
> failing.
>
> I suspect the `MSVCCompat` case may need to be handled differently depending 
> on the host OS similar to `PPDirectives.cpp`:
>
> if (LangOpts.MSVCCompat) {
>   NormalizedPath = Filename.str();
>   #ifndef _WIN32
>   llvm::sys::path::native(NormalizedPath);
>   #endif
> }
>
>
> Reopening this for now, will try to get a local Windows builder set up and 
> work out what the problem is before relanding.


Can't you just always use `llvm::sys::path::filename(PLFileName)`?  It defaults 
to the native style.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61756



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

[PATCH] D61747: [clang-tidy] remove default header-filter for run-clang-tidy

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

I've also had issues with this behavior.  It's a bad default, as the PR notes, 
but is there a way to provide a better one and still respect the config file?

Also, could you add a test for this?  Perhaps in 
`clang-tools-extra/test//clang-tidy/run-clang-tidy.cpp`.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61747



___
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.

I'll be happy to commit for you, but will give it till tomorrow just to make 
sure no one else has any additional comments.

Thanks again!




Comment at: 
clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst:262
+not been used on code with a compatibility requirements of OpenMP prior to
+version 5. It is **intentional** that this check does not make any attempt to
+not issue diagnostics on OpenMP for loops.

Could you reword this last line to remove the double negative?  Perhaps 
something like:

```
This check makes no attempt to exclude incorrect diagnostics for OpenMP loops 
prior to OpenMP 5.
```


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 accepted this revision.
hintonda added a comment.
This revision is now accepted and ready to land.

Awesome, thanks...

LGTM, but you may want to give @alexfh or @aaron.ballman a day to comment 
before you commit.


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#1499368 , @lebedev.ri wrote:

> In D61827#1499347 , @hintonda wrote:
>
> >
>




>> Are you saying this patch is a regression?
> 
> Not in general, no. This is most certainly an improvement for normal C++ code.

Good, then this patch can go forward.

I agree that a note that this shouldn't be run on openmp 4.  Perhaps a separate 
patch could check for that and not do anything when compiling an openmp file.


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.

I pulled down you patch, compiled and ran it.  Once I fixed the two problems I 
mentioned, it ran clean, e.g.:

  $ bin/llvm-lit -vv 
../../llvm-project/clang-tools-extra/test/clang-tidy/modernize-loop-convert-[be]*.cpp
  -- Testing: 2 tests, 2 threads --
  PASS: Clang Tools :: clang-tidy/modernize-loop-convert-basic.cpp (1 of 2)
  PASS: Clang Tools :: clang-tidy/modernize-loop-convert-extra.cpp (2 of 2)
  Testing Time: 0.93s
Expected Passes: 2

Also, I don't think the omp issue is a regression, so I wouldn't worry about 
trying to fix it here.




Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp:131
   DeclarationMatcher InitDeclMatcher =
-  varDecl(hasInitializer(anyOf(ignoringParenImpCasts(BeginCallMatcher),
-   materializeTemporaryExpr(

This change is incorrect.  The matcher actually needs all three cases.  This 
explains the strange test failures you were seeing.



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

Instead of adding a new test case, please fix the two I mentioned earlier, and 
5 more in `modernize-loop-convert-extra.cpp`.  They just need you to add the 
appropriate CHECK line.


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#1499335 , @lebedev.ri wrote:

> In D61827#1499333 , @hintonda wrote:
>
> > When I asked for a test above, I understood you to say you couldn't provide 
> > one, but If I misunderstood, by all means, please add the test.
>
>
> Please do note that i have provided a testcase (godbolt link) in my very 
> first comment, and quoted that line when replying the previous time.
>  (Granted, that loop is not in a correct form for openmp, but the point 
> being, the current check does not diagnose it either)


I'm really not sure I understand what you are trying to say.  Are you saying 
this patch is a regression?

By "false positive", do you mean that diagnoses something it shouldn't change 
and creates an erroneous fixit?  Of that it ignores something it should fix?


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#1499309 , @lebedev.ri wrote:

> In D61827#1499306 , @hintonda wrote:
>
> > In D61827#1499303 , @torbjoernk 
> > wrote:
> >
> > > In D61827#1499184 , @lebedev.ri 
> > > wrote:
> > >
> > > > 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.
> > >
> > >
> > > I have to admit that I have very little experience with OpenMP and 
> > > haven't thought of this at all. Thank you very much for bringing this up.
> > >
> > > Would it help to extend the exclusion AST matcher for iterator-based 
> > > loops by an exclusion for loops with an ancestor of 
> > > `ompExecutableDirective`?:
> > >
> > >   return forStmt(
> > >unless(anyOf(isInTemplateInstantiation(),
> > > hasAncestor(ompExecutableDirective(,
> > >
> >
> >
> > As a general rule, don't add anything that doesn't include a test.
> >
> > Since this "false positive" is apparently untestable,
>
>
> How so?


When I asked for a test above, I understood you to say you couldn't provide 
one, but If I misunderstood, by all means, please add the test.


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#1499303 , @torbjoernk wrote:

> In D61827#1499184 , @lebedev.ri 
> wrote:
>
> > 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.
>
>
> I have to admit that I have very little experience with OpenMP and haven't 
> thought of this at all. Thank you very much for bringing this up.
>
> Would it help to extend the exclusion AST matcher for iterator-based loops by 
> an exclusion for loops with an ancestor of `ompExecutableDirective`?:
>
>   return forStmt(
>unless(anyOf(isInTemplateInstantiation(),
> hasAncestor(ompExecutableDirective(,
>


As a general rule, don't add anything that doesn't include a test.

Since this "false positive" is apparently untestable, as far as I'm concerned, 
it doesn't exist.  Most tests of this sort are mocked up to emulate the problem 
so you can verify it exists and the fix actually addresses it.


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

[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] D61697: [lit] Disable test on darwin when building shared libs.

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

@zturner, I only disabled this on Darwin because that's all I can test on right 
now.  Please let me know if there's a better way to do this or if it should 
fire on different platforms.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61697



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

[PATCH] D61697: [lit] Disable test on darwin when building shared libs.

[Don Hinton via Phabricator via cfe-commits]

hintonda created this revision.
hintonda added a reviewer: zturner.
Herald added subscribers: llvm-commits, delcypher.
Herald added projects: clang, LLVM.

This test fails to link shared libraries because tries to run
a copied version of clang-check to see if the mock version of libcxx
in the same directory can be loaded dynamically.  Since the test is
specifically designed not to look in the default just-built lib
directory, it must be disabled when building with
BUILD_SHARED_LIBS=ON.

Currently only disabling it on Darwin and basing it on the
enable_shared flag.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61697

Files:
  clang/test/Tooling/clang-check-mac-libcxx-fixed-compilation-db.cpp
  llvm/utils/lit/lit/llvm/config.py


Index: llvm/utils/lit/lit/llvm/config.py
===
--- llvm/utils/lit/lit/llvm/config.py
+++ llvm/utils/lit/lit/llvm/config.py
@@ -52,6 +52,8 @@
 # We should standardize on the former.
 features.add('system-linker-mach-o')
 features.add('system-darwin')
+if config.enable_shared:
+features.add("enable_shared")
 elif platform.system() == 'Windows':
 # For tests that require Windows to run.
 features.add('system-windows')
Index: clang/test/Tooling/clang-check-mac-libcxx-fixed-compilation-db.cpp
===
--- clang/test/Tooling/clang-check-mac-libcxx-fixed-compilation-db.cpp
+++ clang/test/Tooling/clang-check-mac-libcxx-fixed-compilation-db.cpp
@@ -16,5 +16,7 @@
 //
 // ^ -ccc-install-dir passed to unbreak tests on *BSD where
 //   getMainExecutable() relies on real argv[0] being passed
+//
+// UNSUPPORTED: enable_shared
 #include 
 vector v;


Index: llvm/utils/lit/lit/llvm/config.py
===
--- llvm/utils/lit/lit/llvm/config.py
+++ llvm/utils/lit/lit/llvm/config.py
@@ -52,6 +52,8 @@
 # We should standardize on the former.
 features.add('system-linker-mach-o')
 features.add('system-darwin')
+if config.enable_shared:
+features.add("enable_shared")
 elif platform.system() == 'Windows':
 # For tests that require Windows to run.
 features.add('system-windows')
Index: clang/test/Tooling/clang-check-mac-libcxx-fixed-compilation-db.cpp
===
--- clang/test/Tooling/clang-check-mac-libcxx-fixed-compilation-db.cpp
+++ clang/test/Tooling/clang-check-mac-libcxx-fixed-compilation-db.cpp
@@ -16,5 +16,7 @@
 //
 // ^ -ccc-install-dir passed to unbreak tests on *BSD where
 //   getMainExecutable() relies on real argv[0] being passed
+//
+// UNSUPPORTED: enable_shared
 #include 
 vector v;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_check'

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D60629#1494908 , @aaron.ballman 
wrote:

> This LGTM, but you may want to wait a day or so to see if @alexfh has any 
> remaining concerns.


Will do, thanks Aaron...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60629



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

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_check'

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

ping...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60629



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

[PATCH] D61269: [CommandLine] Change help output to prefix long options with `--` instead of `-`. NFC . Part 3 of 5

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D61269#1490176 , @thakis wrote:

> I happened to see this go by. Is there an explanation of the overall goal 
> somewhere? In general, requiring -- for long flags sounds like a great change 
> to me, but there are a few exceptions: For example. lld-link should keep 
> accepting long flags with a single dash for link.exe compatibility.


The 5th and final patch, D61294 , 
"[CommandLine] Add long option flag for cl::ParseCommandLineOptions . Part 5 of 
5", adds a flag that requires long options use the `--`.  It's an opt-in 
strategy, so only tools that want the GNU `getopt_long` behavior would want to 
set it.  Otherwise, everything works the way it does currently, with the 
exception of help printing `--` for long options.

As noted in D61294 , these changes were 
motivated by this discussion:  
http://lists.llvm.org/pipermail/llvm-dev/2019-April/131786.html

Of course, this doesn't change tblgen generated options which are a different 
animal.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61269



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

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_check'

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

Ping.  Is this the direction you'd like to go?  Or would you prefer something 
else?  thanks... don


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60629



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

[PATCH] D61270: [CommandLine] Enable Grouping for short options by default. Part 4 of 5

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D61270#1483661 , @MaskRay wrote:

> Thank you for the patch series!
>
> >   This is consistent with GNU getopt behavior.
>
> This is not GNU specific :) It is POSIX Utility Syntax Guidelines 5 
> http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html


Thanks for pointing that out.

While not mentioned here, this series of patches also deals with long options 
which I believe is a GNU thing -- at least I think that's true.  In any case, 
the ultimate goal of these patches is to allow GNU compatibility.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61270



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

[PATCH] D61269: [CommandLine] Change help output to prefix long options with `--` instead of `-`. NFC . Part 3 of 5

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 197466.
hintonda added a comment.

- Refactor and fix additional dashes in error messages and tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61269

Files:
  clang/test/Driver/clang-offload-bundler.c
  llvm/lib/Support/CommandLine.cpp
  llvm/test/FileCheck/dump-input-enable.txt
  llvm/test/Support/check-default-options.txt
  llvm/unittests/Support/CommandLineTest.cpp

Index: llvm/unittests/Support/CommandLineTest.cpp
===
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -1042,7 +1042,7 @@
 
   StackOption TestOption(Opt, cl::desc(HelpText),
   OptionAttributes...);
-  printOptionInfo(TestOption, 25);
+  printOptionInfo(TestOption, 26);
   outs().flush();
 }
 auto Buffer = MemoryBuffer::getFile(File.FilePath);
@@ -1069,8 +1069,8 @@
   cl::values(clEnumValN(OptionValue::Val, "v1", "desc1")));
 
   // clang-format off
-  EXPECT_EQ(Output, ("  -" + Opt + "= - " + HelpText + "\n"
- "=v1-   desc1\n")
+  EXPECT_EQ(Output, ("  --" + Opt + "= - " + HelpText + "\n"
+ "=v1 -   desc1\n")
 .str());
   // clang-format on
 }
@@ -1082,9 +1082,9 @@
 
   // clang-format off
   EXPECT_EQ(Output,
-("  -" + Opt + " - " + HelpText + "\n"
- "  -" + Opt + "= - " + HelpText + "\n"
- "=v1-   desc1\n")
+("  --" + Opt + " - " + HelpText + "\n"
+ "  --" + Opt + "= - " + HelpText + "\n"
+ "=v1 -   desc1\n")
 .str());
   // clang-format on
 }
@@ -1095,10 +1095,10 @@
 clEnumValN(OptionValue::Val, "", "desc2")));
 
   // clang-format off
-  EXPECT_EQ(Output, ("  -" + Opt + " - " + HelpText + "\n"
- "  -" + Opt + "= - " + HelpText + "\n"
- "=v1-   desc1\n"
- "=   -   desc2\n")
+  EXPECT_EQ(Output, ("  --" + Opt + " - " + HelpText + "\n"
+ "  --" + Opt + "= - " + HelpText + "\n"
+ "=v1 -   desc1\n"
+ "=-   desc2\n")
 .str());
   // clang-format on
 }
@@ -1109,8 +1109,8 @@
 clEnumValN(OptionValue::Val, "", "")));
 
   // clang-format off
-  EXPECT_EQ(Output, ("  -" + Opt + "= - " + HelpText + "\n"
- "=v1-   desc1\n"
+  EXPECT_EQ(Output, ("  --" + Opt + "= - " + HelpText + "\n"
+ "=v1 -   desc1\n"
  "=\n")
 .str());
   // clang-format on
@@ -1122,7 +1122,7 @@
 
   // clang-format off
   EXPECT_EQ(Output,
-("  -" + Opt + "= - " + HelpText + "\n"
+("  --" + Opt + "= - " + HelpText + "\n"
  "=v1\n").str());
   // clang-format on
 }
@@ -1147,7 +1147,7 @@
 
 TEST_F(GetOptionWidthTest, GetOptionWidthArgNameLonger) {
   StringRef ArgName("a-long-argument-name");
-  size_t ExpectedStrSize = ("  -" + ArgName + "= - ").str().size();
+  size_t ExpectedStrSize = ("  --" + ArgName + "= - ").str().size();
   EXPECT_EQ(
   runTest(ArgName, cl::values(clEnumValN(OptionValue::Val, "v", "help"))),
   ExpectedStrSize);
Index: llvm/test/Support/check-default-options.txt
===
--- llvm/test/Support/check-default-options.txt
+++ llvm/test/Support/check-default-options.txt
@@ -1,18 +1,18 @@
-# RUN: llvm-objdump -help-hidden %t | FileCheck --check-prefix=CHECK-OBJDUMP %s
-# RUN: llvm-readobj -help-hidden %t | FileCheck --check-prefix=CHECK-READOBJ %s
-# RUN: llvm-tblgen -help-hidden %t | FileCheck --check-prefix=CHECK-TBLGEN %s
-# RUN: llvm-opt-report -help-hidden %t | FileCheck --check-prefix=CHECK-OPT-RPT %s
-# RUN: llvm-dwarfdump -help-hidden %t | FileCheck --check-prefix=CHECK-DWARF %s
+# RUN: llvm-objdump --help-hidden %t | FileCheck --check-prefix=CHECK-OBJDUMP %s
+# RUN: llvm-readobj --help-hidden %t | FileCheck --check-prefix=CHECK-READOBJ %s
+# RUN: llvm-tblgen --help-hidden %t | FileCheck --check-prefix=CHECK-TBLGEN %s
+# RUN: llvm-opt-report --help-hidden %t | FileCheck --check-prefix=CHECK-OPT-RPT %s
+# RUN: llvm-dwarfdump --help-hidden %t | FileCheck --check-prefix=CHECK-DWARF %s
 # RUN: llvm-dwarfdump -h %t | FileCheck --check-prefix=CHECK-DWARF-H %s
 
 
 # CHECK-OBJDUMP: -h  - Alias for --section-headers
 # CHECK-READOBJ: -h  - Alias for --file-headers
-# CHECK-TBLGEN:  -h  - Alias for -help
-# CHECK-OPT-RPT: -h  - Alias for -help
+# CHECK-TBLGEN:  -h  - Alias for --he

[PATCH] D61269: [CommandLine] Change help output to prefix long options with `--` instead of `-`. NFC . Part 3 of 5

[Don Hinton via Phabricator via cfe-commits]

hintonda marked an inline comment as done.
hintonda added inline comments.



Comment at: llvm/lib/Support/CommandLine.cpp:95
+
+static size_t argPrefixesSize(size_t len) {
+  if (len == 1)

thopre wrote:
> Any reason why not take a StringRef of the option to compute the prefix of? 
> That would put the .size() gymnastic in one place instead of many.
Actually, that's what I originally had, but since it was only a few places, I 
changed it to take the length.  Now that I see it's used all over, I'll 
probably switch it back as you suggest.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61269



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

[PATCH] D61270: [CommandLine] Enable Grouping for short options by default. Part 4 of 5

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 197272.
hintonda added a comment.

Minimize diff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61270

Files:
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp
  llvm/test/tools/llvm-readobj/merged.test


Index: llvm/test/tools/llvm-readobj/merged.test
===
--- llvm/test/tools/llvm-readobj/merged.test
+++ llvm/test/tools/llvm-readobj/merged.test
@@ -10,4 +10,4 @@
 RUN: not llvm-readobj -aeWhSrnudlVgIs %p/Inputs/trivial.obj.elf-i386 2>&1 | 
FileCheck %s --check-prefix=UNKNOWN
 
 CHECK-NOT: Unknown command line argument
-UNKNOWN:   Unknown command line argument
+UNKNOWN:   for the --section-headers option: may only occur zero or one times!
Index: llvm/lib/Support/CommandLine.cpp
===
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -408,6 +408,8 @@
 GlobalParser->updateArgStr(this, S);
   assert((S.empty() || S[0] != '-') && "Option can't start with '-");
   ArgStr = S;
+  if (ArgStr.size() == 1)
+setMiscFlag(Grouping);
 }
 
 void Option::reset() {
Index: llvm/include/llvm/Support/CommandLine.h
===
--- llvm/include/llvm/Support/CommandLine.h
+++ llvm/include/llvm/Support/CommandLine.h
@@ -1205,7 +1205,11 @@
 };
 
 template <> struct applicator {
-  static void opt(MiscFlags MF, Option &O) { O.setMiscFlag(MF); }
+  static void opt(MiscFlags MF, Option &O) {
+assert((MF != Grouping || O.ArgStr.size() == 1) &&
+   "cl::Grouping can only apply to single charater Options.");
+O.setMiscFlag(MF);
+  }
 };
 
 // apply method - Apply modifiers to an option in a type safe way.


Index: llvm/test/tools/llvm-readobj/merged.test
===
--- llvm/test/tools/llvm-readobj/merged.test
+++ llvm/test/tools/llvm-readobj/merged.test
@@ -10,4 +10,4 @@
 RUN: not llvm-readobj -aeWhSrnudlVgIs %p/Inputs/trivial.obj.elf-i386 2>&1 | FileCheck %s --check-prefix=UNKNOWN
 
 CHECK-NOT: Unknown command line argument
-UNKNOWN:   Unknown command line argument
+UNKNOWN:   for the --section-headers option: may only occur zero or one times!
Index: llvm/lib/Support/CommandLine.cpp
===
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -408,6 +408,8 @@
 GlobalParser->updateArgStr(this, S);
   assert((S.empty() || S[0] != '-') && "Option can't start with '-");
   ArgStr = S;
+  if (ArgStr.size() == 1)
+setMiscFlag(Grouping);
 }
 
 void Option::reset() {
Index: llvm/include/llvm/Support/CommandLine.h
===
--- llvm/include/llvm/Support/CommandLine.h
+++ llvm/include/llvm/Support/CommandLine.h
@@ -1205,7 +1205,11 @@
 };
 
 template <> struct applicator {
-  static void opt(MiscFlags MF, Option &O) { O.setMiscFlag(MF); }
+  static void opt(MiscFlags MF, Option &O) {
+assert((MF != Grouping || O.ArgStr.size() == 1) &&
+   "cl::Grouping can only apply to single charater Options.");
+O.setMiscFlag(MF);
+  }
 };
 
 // apply method - Apply modifiers to an option in a type safe way.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61270: [CommandLine] Enable Grouping for short options by default. Part 4 of 5

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 197271.
hintonda added a comment.
Herald added subscribers: cfe-commits, thopre.
Herald added a project: clang.

- Fix test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61270

Files:
  clang/test/Driver/clang-offload-bundler.c
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp
  llvm/test/FileCheck/dump-input-enable.txt
  llvm/test/Support/check-default-options.txt
  llvm/test/tools/llvm-readobj/merged.test

Index: llvm/test/tools/llvm-readobj/merged.test
===
--- llvm/test/tools/llvm-readobj/merged.test
+++ llvm/test/tools/llvm-readobj/merged.test
@@ -10,4 +10,4 @@
 RUN: not llvm-readobj -aeWhSrnudlVgIs %p/Inputs/trivial.obj.elf-i386 2>&1 | FileCheck %s --check-prefix=UNKNOWN
 
 CHECK-NOT: Unknown command line argument
-UNKNOWN:   Unknown command line argument
+UNKNOWN:   for the --section-headers option: may only occur zero or one times!
Index: llvm/test/Support/check-default-options.txt
===
--- llvm/test/Support/check-default-options.txt
+++ llvm/test/Support/check-default-options.txt
@@ -1,18 +1,18 @@
-# RUN: llvm-objdump -help-hidden %t | FileCheck --check-prefix=CHECK-OBJDUMP %s
-# RUN: llvm-readobj -help-hidden %t | FileCheck --check-prefix=CHECK-READOBJ %s
-# RUN: llvm-tblgen -help-hidden %t | FileCheck --check-prefix=CHECK-TBLGEN %s
-# RUN: llvm-opt-report -help-hidden %t | FileCheck --check-prefix=CHECK-OPT-RPT %s
-# RUN: llvm-dwarfdump -help-hidden %t | FileCheck --check-prefix=CHECK-DWARF %s
+# RUN: llvm-objdump --help-hidden %t | FileCheck --check-prefix=CHECK-OBJDUMP %s
+# RUN: llvm-readobj --help-hidden %t | FileCheck --check-prefix=CHECK-READOBJ %s
+# RUN: llvm-tblgen --help-hidden %t | FileCheck --check-prefix=CHECK-TBLGEN %s
+# RUN: llvm-opt-report --help-hidden %t | FileCheck --check-prefix=CHECK-OPT-RPT %s
+# RUN: llvm-dwarfdump --help-hidden %t | FileCheck --check-prefix=CHECK-DWARF %s
 # RUN: llvm-dwarfdump -h %t | FileCheck --check-prefix=CHECK-DWARF-H %s
 
 
 # CHECK-OBJDUMP: -h  - Alias for --section-headers
 # CHECK-READOBJ: -h  - Alias for --file-headers
-# CHECK-TBLGEN:  -h  - Alias for -help
-# CHECK-OPT-RPT: -h  - Alias for -help
+# CHECK-TBLGEN:  -h  - Alias for --help
+# CHECK-OPT-RPT: -h  - Alias for --help
 # CHECK-DWARF:   -h  - Alias for -help
 
 # llvm-dwarfdump declares `-h` option and prints special help in that case,
 # which is weird, but makes for a good test, i.e., shows the default `-h`
 # wasn't used.
-# CHECK-DWARF-H-NOT: -help-list  - Display list of available options (-help-list-hidden for more)
+# CHECK-DWARF-H-NOT: --help-list  - Display list of available options (--help-list-hidden for more)
Index: llvm/test/FileCheck/dump-input-enable.txt
===
--- llvm/test/FileCheck/dump-input-enable.txt
+++ llvm/test/FileCheck/dump-input-enable.txt
@@ -26,7 +26,7 @@
 ; RUN: not FileCheck -dump-input=foobar 2>&1 \
 ; RUN: | FileCheck %s -match-full-lines -check-prefix=BADVAL
 
-BADVAL: {{F|f}}ile{{C|c}}heck{{.*}}: for the -dump-input option: Cannot find option named 'foobar'!
+BADVAL: {{F|f}}ile{{C|c}}heck{{.*}}: for the --dump-input option: Cannot find option named 'foobar'!
 
 ;--
 ; Check -dump-input=help.
Index: llvm/lib/Support/CommandLine.cpp
===
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -88,6 +88,22 @@
 
 //===--===//
 
+static StringRef ArgPrefix = "  -";
+static StringRef ArgPrefixLong = "  --";
+static StringRef ArgHelpPrefix = " - ";
+
+static size_t argPrefixesSize(size_t len) {
+  if (len == 1)
+return ArgPrefix.size() + ArgHelpPrefix.size();
+  return ArgPrefixLong.size() + ArgHelpPrefix.size();
+}
+
+static StringRef argPrefix(size_t len) {
+  if (len == 1)
+return ArgPrefix;
+  return ArgPrefixLong;
+}
+
 namespace {
 
 class CommandLineParser {
@@ -392,6 +408,8 @@
 GlobalParser->updateArgStr(this, S);
   assert((S.empty() || S[0] != '-') && "Option can't start with '-");
   ArgStr = S;
+  if (ArgStr.size() == 1)
+setMiscFlag(Grouping);
 }
 
 void Option::reset() {
@@ -1339,12 +1357,13 @@
 if (!Handler) {
   if (SinkOpts.empty()) {
 *Errs << ProgramName << ": Unknown command line argument '" << argv[i]
-  << "'.  Try: '" << argv[0] << " -help'\n";
+  << "'.  Try: '" << argv[0] << " --help'\n";
 
 if (NearestHandler) {
   // If we know a near match, report it as well.
-  *Errs << ProgramName << ": Did you mean '-" << NearestHandlerString
- << "'?\n";
+  *Errs << ProgramName << ": Did you mean '"
+ 

[PATCH] D61269: [CommandLine] Change help output to prefix long options with `--` instead of `-`. NFC . Part 3 of 5

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 197267.
hintonda added a comment.
Herald added subscribers: cfe-commits, thopre.
Herald added a project: clang.

- Fix dashes in error messages and in tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61269

Files:
  clang/test/Driver/clang-offload-bundler.c
  llvm/lib/Support/CommandLine.cpp
  llvm/test/FileCheck/dump-input-enable.txt
  llvm/test/Support/check-default-options.txt

Index: llvm/test/Support/check-default-options.txt
===
--- llvm/test/Support/check-default-options.txt
+++ llvm/test/Support/check-default-options.txt
@@ -1,18 +1,18 @@
-# RUN: llvm-objdump -help-hidden %t | FileCheck --check-prefix=CHECK-OBJDUMP %s
-# RUN: llvm-readobj -help-hidden %t | FileCheck --check-prefix=CHECK-READOBJ %s
-# RUN: llvm-tblgen -help-hidden %t | FileCheck --check-prefix=CHECK-TBLGEN %s
-# RUN: llvm-opt-report -help-hidden %t | FileCheck --check-prefix=CHECK-OPT-RPT %s
-# RUN: llvm-dwarfdump -help-hidden %t | FileCheck --check-prefix=CHECK-DWARF %s
+# RUN: llvm-objdump --help-hidden %t | FileCheck --check-prefix=CHECK-OBJDUMP %s
+# RUN: llvm-readobj --help-hidden %t | FileCheck --check-prefix=CHECK-READOBJ %s
+# RUN: llvm-tblgen --help-hidden %t | FileCheck --check-prefix=CHECK-TBLGEN %s
+# RUN: llvm-opt-report --help-hidden %t | FileCheck --check-prefix=CHECK-OPT-RPT %s
+# RUN: llvm-dwarfdump --help-hidden %t | FileCheck --check-prefix=CHECK-DWARF %s
 # RUN: llvm-dwarfdump -h %t | FileCheck --check-prefix=CHECK-DWARF-H %s
 
 
 # CHECK-OBJDUMP: -h  - Alias for --section-headers
 # CHECK-READOBJ: -h  - Alias for --file-headers
-# CHECK-TBLGEN:  -h  - Alias for -help
-# CHECK-OPT-RPT: -h  - Alias for -help
+# CHECK-TBLGEN:  -h  - Alias for --help
+# CHECK-OPT-RPT: -h  - Alias for --help
 # CHECK-DWARF:   -h  - Alias for -help
 
 # llvm-dwarfdump declares `-h` option and prints special help in that case,
 # which is weird, but makes for a good test, i.e., shows the default `-h`
 # wasn't used.
-# CHECK-DWARF-H-NOT: -help-list  - Display list of available options (-help-list-hidden for more)
+# CHECK-DWARF-H-NOT: --help-list  - Display list of available options (--help-list-hidden for more)
Index: llvm/test/FileCheck/dump-input-enable.txt
===
--- llvm/test/FileCheck/dump-input-enable.txt
+++ llvm/test/FileCheck/dump-input-enable.txt
@@ -26,7 +26,7 @@
 ; RUN: not FileCheck -dump-input=foobar 2>&1 \
 ; RUN: | FileCheck %s -match-full-lines -check-prefix=BADVAL
 
-BADVAL: {{F|f}}ile{{C|c}}heck{{.*}}: for the -dump-input option: Cannot find option named 'foobar'!
+BADVAL: {{F|f}}ile{{C|c}}heck{{.*}}: for the --dump-input option: Cannot find option named 'foobar'!
 
 ;--
 ; Check -dump-input=help.
Index: llvm/lib/Support/CommandLine.cpp
===
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -88,6 +88,22 @@
 
 //===--===//
 
+static StringRef ArgPrefix = "  -";
+static StringRef ArgPrefixLong = "  --";
+static StringRef ArgHelpPrefix = " - ";
+
+static size_t argPrefixesSize(size_t len) {
+  if (len == 1)
+return ArgPrefix.size() + ArgHelpPrefix.size();
+  return ArgPrefixLong.size() + ArgHelpPrefix.size();
+}
+
+static StringRef argPrefix(size_t len) {
+  if (len == 1)
+return ArgPrefix;
+  return ArgPrefixLong;
+}
+
 namespace {
 
 class CommandLineParser {
@@ -1339,12 +1355,13 @@
 if (!Handler) {
   if (SinkOpts.empty()) {
 *Errs << ProgramName << ": Unknown command line argument '" << argv[i]
-  << "'.  Try: '" << argv[0] << " -help'\n";
+  << "'.  Try: '" << argv[0] << " --help'\n";
 
 if (NearestHandler) {
   // If we know a near match, report it as well.
-  *Errs << ProgramName << ": Did you mean '-" << NearestHandlerString
- << "'?\n";
+  *Errs << ProgramName << ": Did you mean '"
+<< argPrefix(NearestHandler->ArgStr.size())
+<< NearestHandlerString << "'?\n";
 }
 
 ErrorParsing = true;
@@ -1378,14 +1395,14 @@
  << ": Not enough positional command line arguments specified!\n"
  << "Must specify at least " << NumPositionalRequired
  << " positional argument" << (NumPositionalRequired > 1 ? "s" : "")
- << ": See: " << argv[0] << " -help\n";
+ << ": See: " << argv[0] << " --help\n";
 
 ErrorParsing = true;
   } else if (!HasUnlimitedPositionals &&
  PositionalVals.size() > PositionalOpts.size()) {
 *Errs << ProgramName << ": Too many positional arguments specified!\n"
   << "Can specify at most " << PositionalOpts.size()
-  

[PATCH] D60910: [WIP] Dumping the AST to JSON

[Don Hinton via Phabricator via cfe-commits]

hintonda added inline comments.



Comment at: include/clang/Frontend/FrontendOptions.h:311
+  /// Specifies the output format of the AST.
+  enum ASTOutputFormat {
+AOF_Default,

aaron.ballman wrote:
> hintonda wrote:
> > Why can't you use the enum defined in `clang/AST/ASTDumperUtils.h`?  Seems 
> > to make the `dump()` call below unnecessarily ugly.
> I thought there would be a layering violation if AST and Frontend colluded on 
> that, but I see now that Frontend already links against AST, so perhaps there 
> isn't a layering violation after all?
> 
> I'll investigate to see if this can be cleaned up.
I think that Frontend uses AST, e.g., ASTContext, but not the other way around.


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

https://reviews.llvm.org/D60910



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

[PATCH] D60910: [WIP] Dumping the AST to JSON

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

Looks good, but just a couple of questions.




Comment at: include/clang/AST/JSONNodeDumper.h:73
+  Indentation.clear();
+  OS << "\n" << Indentation << "}\n";
+  TopLevel = true;

Just curious, since you clear Indentation on the previous line, do you need it 
here?



Comment at: include/clang/Frontend/FrontendOptions.h:311
+  /// Specifies the output format of the AST.
+  enum ASTOutputFormat {
+AOF_Default,

Why can't you use the enum defined in `clang/AST/ASTDumperUtils.h`?  Seems to 
make the `dump()` call below unnecessarily ugly.


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

https://reviews.llvm.org/D60910



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

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_check'

[Don Hinton via Phabricator via cfe-commits]

hintonda marked an inline comment as done.
hintonda added inline comments.



Comment at: clang-tools-extra/clang-tidy/rename_check.py:267
 
-  if old_module != new_module:
+  if old_module != new_module or new_module == 'llvm':
+if new_module == 'llvm':

alexfh wrote:
> I believe, we should first construct the new namespace name (using exactly 
> the same code as in add_new_check.py) and then check if it differs from the 
> old one.
Changed this back to the original check which was clearer and easier for the 
reader to grok.

We are checking two distinct things here, so be explicit.  First, we check to 
see if the modules changed, e.g., from say from 'google' to 'modernize', then 
we check to see if the new module is 'llvm' and needs special handling.

Then, and only then, do we need to set the new_namespace variable, and we 
condition that on whether or not the new module name is 'llvm'.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60629



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

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_check'

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 196531.
hintonda added a comment.

- Change check back to previous version which is much clearer.
- Apply namespace change to new check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60629

Files:
  clang-tools-extra/clang-tidy/add_new_check.py
  clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.cpp
  clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.h
  clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp
  clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.h
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp
  clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.h
  clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.cpp
  clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.h
  clang-tools-extra/clang-tidy/rename_check.py
  clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
@@ -3,7 +3,7 @@
 #include "llvm/IncludeOrderCheck.h"
 #include "gtest/gtest.h"
 
-using namespace clang::tidy::llvm;
+using namespace clang::tidy::llvm_check;
 
 namespace clang {
 namespace tidy {
Index: clang-tools-extra/clang-tidy/rename_check.py
===
--- clang-tools-extra/clang-tidy/rename_check.py
+++ clang-tools-extra/clang-tidy/rename_check.py
@@ -14,6 +14,18 @@
 import re
 
 
+def replaceInFileRegex(fileName, sFrom, sTo):
+  if sFrom == sTo:
+return
+  txt = None
+  with open(fileName, "r") as f:
+txt = f.read()
+
+  txt = re.sub(sFrom, sTo, txt)
+  print("Replacing '%s' -> '%s' in '%s'..." % (sFrom, sTo, fileName))
+  with open(fileName, "w") as f:
+f.write(txt)
+
 def replaceInFile(fileName, sFrom, sTo):
   if sFrom == sTo:
 return
@@ -203,6 +215,8 @@
   clang_tidy_path = os.path.dirname(__file__)
 
   header_guard_variants = [
+  (args.old_check_name.replace('-', '_')).upper() + '_CHECK',
+  (old_module + '_' + check_name_camel).upper(),
   (old_module + '_' + new_check_name_camel).upper(),
   args.old_check_name.replace('-', '_').upper()]
   header_guard_new = (new_module + '_' + new_check_name_camel).upper()
@@ -210,18 +224,19 @@
   old_module_path = os.path.join(clang_tidy_path, old_module)
   new_module_path = os.path.join(clang_tidy_path, new_module)
 
-  # Remove the check from the old module.
-  cmake_lists = os.path.join(old_module_path, 'CMakeLists.txt')
-  check_found = deleteMatchingLines(cmake_lists, '\\b' + check_name_camel)
-  if not check_found:
-print("Check name '%s' not found in %s. Exiting." %
+  if (args.old_check_name != args.new_check_name):
+# Remove the check from the old module.
+cmake_lists = os.path.join(old_module_path, 'CMakeLists.txt')
+check_found = deleteMatchingLines(cmake_lists, '\\b' + check_name_camel)
+if not check_found:
+  print("Check name '%s' not found in %s. Exiting." %
 (check_name_camel, cmake_lists))
-return 1
+  return 1
 
-  modulecpp = filter(
-  lambda p: p.lower() == old_module.lower() + 'tidymodule.cpp',
-  os.listdir(old_module_path))[0]
-  deleteMatchingLines(os.path.join(old_module_path, modulecpp),
+modulecpp = filter(
+lambda p: p.lower() == old_module.lower() + 'tidymodule.cpp',
+os.listdir(old_module_path))[0]
+deleteMatchingLines(os.path.join(old_module_path, modulecpp),
   '\\b' + check_name_camel + '|\\b' + args.old_check_name)
 
   for filename in getListOfFiles(clang_tidy_path):
@@ -249,14 +264,21 @@
   new_module + '/' + new_check_name_camel)
 replaceInFile(filename, check_name_camel, new_check_name_camel)
 
-  if old_module != new_module:
+  if old_module != new_module or new_module == 'llvm':
+if new_module == 'llvm':
+  new_namespace = new_module + '_check'
+else:
+  new_namespace = new_module
 check_implementation_files = glob.glob(
 os.path.join(old_module_path, new_check_name_camel + '*'))
 for filename in check_implementation_files:
   # Move check implementation to the directory of the new module.
   filename = fileRename(filename, old_module_path, new_module_path)
-  replaceInFile(filename, 'namespace ' + old_module,
-'namespace ' + new_module)
+  replaceInFileRegex(filename, 'namespace ' + old_module + '[^ \n]*',
+ 'namespace ' + new_namespace)
+
+  if (args.old_check_name == args.new_check_name):
+return
 
   # Add check to the new module.
   adapt_cmake(new_module_path, new_check_name_camel)
Index: clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.h
=

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_check'

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 196521.
hintonda added a comment.

- Remove unnecessary comparison.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60629

Files:
  clang-tools-extra/clang-tidy/add_new_check.py
  clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.cpp
  clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.h
  clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp
  clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.h
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.cpp
  clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.h
  clang-tools-extra/clang-tidy/rename_check.py
  clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
@@ -3,7 +3,7 @@
 #include "llvm/IncludeOrderCheck.h"
 #include "gtest/gtest.h"
 
-using namespace clang::tidy::llvm;
+using namespace clang::tidy::llvm_check;
 
 namespace clang {
 namespace tidy {
Index: clang-tools-extra/clang-tidy/rename_check.py
===
--- clang-tools-extra/clang-tidy/rename_check.py
+++ clang-tools-extra/clang-tidy/rename_check.py
@@ -14,6 +14,18 @@
 import re
 
 
+def replaceInFileRegex(fileName, sFrom, sTo):
+  if sFrom == sTo:
+return
+  txt = None
+  with open(fileName, "r") as f:
+txt = f.read()
+
+  txt = re.sub(sFrom, sTo, txt)
+  print("Replacing '%s' -> '%s' in '%s'..." % (sFrom, sTo, fileName))
+  with open(fileName, "w") as f:
+f.write(txt)
+
 def replaceInFile(fileName, sFrom, sTo):
   if sFrom == sTo:
 return
@@ -203,6 +215,8 @@
   clang_tidy_path = os.path.dirname(__file__)
 
   header_guard_variants = [
+  (args.old_check_name.replace('-', '_')).upper() + '_CHECK',
+  (old_module + '_' + check_name_camel).upper(),
   (old_module + '_' + new_check_name_camel).upper(),
   args.old_check_name.replace('-', '_').upper()]
   header_guard_new = (new_module + '_' + new_check_name_camel).upper()
@@ -210,18 +224,19 @@
   old_module_path = os.path.join(clang_tidy_path, old_module)
   new_module_path = os.path.join(clang_tidy_path, new_module)
 
-  # Remove the check from the old module.
-  cmake_lists = os.path.join(old_module_path, 'CMakeLists.txt')
-  check_found = deleteMatchingLines(cmake_lists, '\\b' + check_name_camel)
-  if not check_found:
-print("Check name '%s' not found in %s. Exiting." %
+  if (args.old_check_name != args.new_check_name):
+# Remove the check from the old module.
+cmake_lists = os.path.join(old_module_path, 'CMakeLists.txt')
+check_found = deleteMatchingLines(cmake_lists, '\\b' + check_name_camel)
+if not check_found:
+  print("Check name '%s' not found in %s. Exiting." %
 (check_name_camel, cmake_lists))
-return 1
+  return 1
 
-  modulecpp = filter(
-  lambda p: p.lower() == old_module.lower() + 'tidymodule.cpp',
-  os.listdir(old_module_path))[0]
-  deleteMatchingLines(os.path.join(old_module_path, modulecpp),
+modulecpp = filter(
+lambda p: p.lower() == old_module.lower() + 'tidymodule.cpp',
+os.listdir(old_module_path))[0]
+deleteMatchingLines(os.path.join(old_module_path, modulecpp),
   '\\b' + check_name_camel + '|\\b' + args.old_check_name)
 
   for filename in getListOfFiles(clang_tidy_path):
@@ -249,14 +264,21 @@
   new_module + '/' + new_check_name_camel)
 replaceInFile(filename, check_name_camel, new_check_name_camel)
 
+  if new_module == 'llvm':
+new_namespace = new_module + '_check'
+  else:
+new_namespace = new_module
   if old_module != new_module:
 check_implementation_files = glob.glob(
 os.path.join(old_module_path, new_check_name_camel + '*'))
 for filename in check_implementation_files:
   # Move check implementation to the directory of the new module.
   filename = fileRename(filename, old_module_path, new_module_path)
-  replaceInFile(filename, 'namespace ' + old_module,
-'namespace ' + new_module)
+  replaceInFileRegex(filename, 'namespace ' + old_module + '[^ \n]*',
+ 'namespace ' + new_namespace)
+
+  if (args.old_check_name == args.new_check_name):
+return
 
   # Add check to the new module.
   adapt_cmake(new_module_path, new_check_name_camel)
Index: clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.h
===
--- clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.h
+++ clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.h
@@ -6,14 +6,14 @@
 //
 //===--===//
 
-#ifndef LLVM_

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D59802#1475596 , @aaron.ballman 
wrote:

> In D59802#1474300 , @hintonda wrote:
>
> > @aaron.ballman, I just ran it over llvm/lib, including all in-tree headers, 
> > and it seems to work fine.  However, it did miss this one:
> >
> > - if (V && isa(V) && (EntInst = cast(V)) && + 
> >if (isa_and_nonnull(V) && (EntInst = cast(V)) 
> > &&
> >
> >   It got the first, but not the second.  Not sure how to pick that one up.  
> > Even ran it a second time on just that file, but still didn't pick it up.  
> > Any ideas?
>
>
> I don't think it's a critical case to cover for the check, but yeah, it looks 
> like that code really wants to be `(EntInst = 
> dyn_cast_or_null(V))`. I think that looking for a pattern to 
> handle this case would be tricky though and given how infrequent it seems to 
> show up in the code base, I am not too worried. Someone applying the check 
> will still get a notice for the `Var && isa(Var)` part of the 
> expression, so they can hopefully see there's more related cleanup to be done.


Okay, thanks for looking.  I'll go ahead and land this today, and if I think of 
a good way to handle this case, I'll update it later.

Thanks again for all your help...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

@aaron.ballman, I just ran it over llvm/lib, including all in-tree headers, and 
it seems to work fine.  However, it did miss this one:

- if (V && isa(V) && (EntInst = cast(V)) &&

+if (isa_and_nonnull(V) && (EntInst = 
cast(V)) &&

It got the first, but not the second.  Not sure how to pick that one up.  Even 
ran it a second time on just that file, but still didn't pick it up.  Any ideas?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D59754: [Sema] Add c++2a designated initializer warnings

[Don Hinton via Phabricator via cfe-commits]

hintonda marked an inline comment as done.
hintonda added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:2069-2072
+if (!VerifyOnly && HasDesignatedInit && SemaRef.getLangOpts().CPlusPlus2a) 
{
+  SemaRef.Diag(Init->getBeginLoc(), diag::ext_c20_designated_init)
+  << "mixed" << Init->getSourceRange();
+}

hintonda wrote:
> rsmith wrote:
> > I think it would be preferable to diagnose the "mixed" case in the parser 
> > rather than here (it's a grammatical restriction in C++, after all). I'm 
> > worried that handling it here will get some cases wrong, such as perhaps:
> > 
> > ```
> > struct A {
> >   union { int x, y; };
> >   int z;
> > };
> > A a = { .x = 123, 456 }; // should be rejected, but I think this patch 
> > might accept
> > ```
> > 
> > ... and it might also get template instantiation cases wrong for a case 
> > like:
> > 
> > ```
> > struct Q { Q(); };
> > struct A { Q x, y; };
> > template void f() {
> >   A a = {.y = Q()};
> > }
> > void g() { f(); }
> > ```
> > 
> > ... where we might possibly end up passing an `InitListExpr` representing 
> > `{Q(), .y = Q()}` into `InitListChecker`.
> > 
> > I think the only C++20 restriction that it makes sense to check in 
> > `InitListChecker` is that the field names are in the right order; 
> > everything else should be checked earlier. This would also match the 
> > semantics of overload resolution, for which the "fields are in the right 
> > order" check is deferred until after a function is selected, whereas all 
> > the other checks are performed eagerly.
> Will work on moving these to the parser.
> 
> Btw, the first one is diagnosed correctly, but doesn't complain about the 
> second.  Not sure I grok the problem there either since Q has a default ctor.
> 
Woke up this morning and realized what you meant about the union.  I'll take 
care of it in the next patch.

thanks again...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59754



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

[PATCH] D59754: [Sema] Add c++2a designated initializer warnings

[Don Hinton via Phabricator via cfe-commits]

hintonda marked an inline comment as done.
hintonda added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:2069-2072
+if (!VerifyOnly && HasDesignatedInit && SemaRef.getLangOpts().CPlusPlus2a) 
{
+  SemaRef.Diag(Init->getBeginLoc(), diag::ext_c20_designated_init)
+  << "mixed" << Init->getSourceRange();
+}

rsmith wrote:
> I think it would be preferable to diagnose the "mixed" case in the parser 
> rather than here (it's a grammatical restriction in C++, after all). I'm 
> worried that handling it here will get some cases wrong, such as perhaps:
> 
> ```
> struct A {
>   union { int x, y; };
>   int z;
> };
> A a = { .x = 123, 456 }; // should be rejected, but I think this patch might 
> accept
> ```
> 
> ... and it might also get template instantiation cases wrong for a case like:
> 
> ```
> struct Q { Q(); };
> struct A { Q x, y; };
> template void f() {
>   A a = {.y = Q()};
> }
> void g() { f(); }
> ```
> 
> ... where we might possibly end up passing an `InitListExpr` representing 
> `{Q(), .y = Q()}` into `InitListChecker`.
> 
> I think the only C++20 restriction that it makes sense to check in 
> `InitListChecker` is that the field names are in the right order; everything 
> else should be checked earlier. This would also match the semantics of 
> overload resolution, for which the "fields are in the right order" check is 
> deferred until after a function is selected, whereas all the other checks are 
> performed eagerly.
Will work on moving these to the parser.

Btw, the first one is diagnosed correctly, but doesn't complain about the 
second.  Not sure I grok the problem there either since Q has a default ctor.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59754



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

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_check'

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 196011.
hintonda added a comment.

- Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60629

Files:
  clang-tools-extra/clang-tidy/add_new_check.py
  clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.cpp
  clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.h
  clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp
  clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.h
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.cpp
  clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.h
  clang-tools-extra/clang-tidy/rename_check.py
  clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
@@ -3,7 +3,7 @@
 #include "llvm/IncludeOrderCheck.h"
 #include "gtest/gtest.h"
 
-using namespace clang::tidy::llvm;
+using namespace clang::tidy::llvm_check;
 
 namespace clang {
 namespace tidy {
Index: clang-tools-extra/clang-tidy/rename_check.py
===
--- clang-tools-extra/clang-tidy/rename_check.py
+++ clang-tools-extra/clang-tidy/rename_check.py
@@ -14,6 +14,18 @@
 import re
 
 
+def replaceInFileRegex(fileName, sFrom, sTo):
+  if sFrom == sTo:
+return
+  txt = None
+  with open(fileName, "r") as f:
+txt = f.read()
+
+  txt = re.sub(sFrom, sTo, txt)
+  print("Replacing '%s' -> '%s' in '%s'..." % (sFrom, sTo, fileName))
+  with open(fileName, "w") as f:
+f.write(txt)
+
 def replaceInFile(fileName, sFrom, sTo):
   if sFrom == sTo:
 return
@@ -203,6 +215,8 @@
   clang_tidy_path = os.path.dirname(__file__)
 
   header_guard_variants = [
+  (args.old_check_name.replace('-', '_')).upper() + '_CHECK',
+  (old_module + '_' + check_name_camel).upper(),
   (old_module + '_' + new_check_name_camel).upper(),
   args.old_check_name.replace('-', '_').upper()]
   header_guard_new = (new_module + '_' + new_check_name_camel).upper()
@@ -210,18 +224,19 @@
   old_module_path = os.path.join(clang_tidy_path, old_module)
   new_module_path = os.path.join(clang_tidy_path, new_module)
 
-  # Remove the check from the old module.
-  cmake_lists = os.path.join(old_module_path, 'CMakeLists.txt')
-  check_found = deleteMatchingLines(cmake_lists, '\\b' + check_name_camel)
-  if not check_found:
-print("Check name '%s' not found in %s. Exiting." %
+  if (args.old_check_name != args.new_check_name):
+# Remove the check from the old module.
+cmake_lists = os.path.join(old_module_path, 'CMakeLists.txt')
+check_found = deleteMatchingLines(cmake_lists, '\\b' + check_name_camel)
+if not check_found:
+  print("Check name '%s' not found in %s. Exiting." %
 (check_name_camel, cmake_lists))
-return 1
+  return 1
 
-  modulecpp = filter(
-  lambda p: p.lower() == old_module.lower() + 'tidymodule.cpp',
-  os.listdir(old_module_path))[0]
-  deleteMatchingLines(os.path.join(old_module_path, modulecpp),
+modulecpp = filter(
+lambda p: p.lower() == old_module.lower() + 'tidymodule.cpp',
+os.listdir(old_module_path))[0]
+deleteMatchingLines(os.path.join(old_module_path, modulecpp),
   '\\b' + check_name_camel + '|\\b' + args.old_check_name)
 
   for filename in getListOfFiles(clang_tidy_path):
@@ -249,14 +264,21 @@
   new_module + '/' + new_check_name_camel)
 replaceInFile(filename, check_name_camel, new_check_name_camel)
 
-  if old_module != new_module:
+  if new_module == 'llvm':
+new_namespace = new_module + '_check'
+  else:
+new_namespace = new_module
+  if old_module != new_module or new_namespace == 'llvm_check':
 check_implementation_files = glob.glob(
 os.path.join(old_module_path, new_check_name_camel + '*'))
 for filename in check_implementation_files:
   # Move check implementation to the directory of the new module.
   filename = fileRename(filename, old_module_path, new_module_path)
-  replaceInFile(filename, 'namespace ' + old_module,
-'namespace ' + new_module)
+  replaceInFileRegex(filename, 'namespace ' + old_module + '[^ \n]*',
+ 'namespace ' + new_namespace)
+
+  if (args.old_check_name == args.new_check_name):
+return
 
   # Add check to the new module.
   adapt_cmake(new_module_path, new_check_name_camel)
Index: clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.h
===
--- clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.h
+++ clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.h
@@ -6,14 +6,14 @@
 //
 //===

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_check'

[Don Hinton via Phabricator via cfe-commits]

hintonda marked an inline comment as done.
hintonda added inline comments.



Comment at: clang-tools-extra/clang-tidy/rename_check.py:218
   header_guard_variants = [
+  (args.old_check_name.replace('-', '_') + '_Check').upper(),
+  (old_module + '_' + check_name_camel).upper(),

alexfh wrote:
> s/_Check/_CHECK/, maybe?
Not sure it matters, but args.old_check_name is all lower case, so we have to 
uppercase the entire string.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60629



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

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

[Don Hinton via Phabricator via cfe-commits]

hintonda added inline comments.



Comment at: clang-tidy/abseil/AbseilTidyModule.cpp:77
+Opts["abseil-make-unique.MakeSmartPtrFunction"] = "absl::make_unique";
+Opts["abseil-make-unique.IgnoreListInit"] = true;
+return Options;

This is defined as `typedef std::map OptionMap;`, so 
you need to assign a string, not a literal `true` value.  hth...


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

https://reviews.llvm.org/D55044



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

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

[Don Hinton via Phabricator via cfe-commits]

hintonda added inline comments.



Comment at: clang-tidy/modernize/MakeSmartPtrCheck.cpp:69
+  IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)),
+  IgnoreListInit(Options.get("IgnoreListInit", false)) {}
 

hintonda wrote:
> axzhang wrote:
> > hintonda wrote:
> > > You’re setting it false here.
> > I thought that false was the default value if the options do not contain 
> > "IgnoreListInit"?
> Do you have a test that passes this option?  I didn’t see one.
I could be wrong, but when you do an Options.get(), that looks at what was 
passed on the commandline, and if it wasn't found, it sets the default provided.


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

https://reviews.llvm.org/D55044



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

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

[Don Hinton via Phabricator via cfe-commits]

hintonda added inline comments.



Comment at: clang-tidy/modernize/MakeSmartPtrCheck.cpp:69
+  IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)),
+  IgnoreListInit(Options.get("IgnoreListInit", false)) {}
 

axzhang wrote:
> hintonda wrote:
> > You’re setting it false here.
> I thought that false was the default value if the options do not contain 
> "IgnoreListInit"?
Do you have a test that passes this option?  I didn’t see one.


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

https://reviews.llvm.org/D55044



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

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

[Don Hinton via Phabricator via cfe-commits]

hintonda added inline comments.



Comment at: clang-tidy/modernize/MakeSmartPtrCheck.cpp:69
+  IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)),
+  IgnoreListInit(Options.get("IgnoreListInit", false)) {}
 

You’re setting it false here.


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

https://reviews.llvm.org/D55044



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

[PATCH] D59754: [Sema] Add c++2a designated initializer warnings

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 195611.
hintonda added a comment.

- Fix typo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59754

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaInit.cpp
  clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp

Index: clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
===
--- clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
+++ clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++2a %s -verify
+// RUN: %clang_cc1 -std=c++2a %s -verify -pedantic
 
 namespace class_with_ctor {
   struct A { // expected-note 6{{candidate}}
@@ -21,3 +21,14 @@
   C c1 = {{}, {}}; // ok, call default ctor twice
   C c2 = {{1, 2}, {3, 4}}; // expected-error 2{{no matching constructor}}
 }
+
+namespace designator {
+struct A { int x, y; };
+struct B { A a; };
+
+A a1 = {.y = 1, .x = 2}; // expected-warning {{C99 out of order designated initializers are a C++20 extension}}
+int arr[3] = {[1] = 5}; // expected-warning {{C99 array designated initializers are a C++20 extension}}
+B b = {.a.x = 0}; // expected-warning {{C99 nested designated initializers are a C++20 extension}}
+A a2 = {.x = 1, 2}; // expected-warning {{C99 mixed designated initializers are a C++20 extension}}
+A a3 = {1, .y = 2}; // expected-warning {{C99 mixed designated initializers are a C++20 extension}}
+}
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -1999,6 +1999,7 @@
   bool CheckForMissingFields =
 !IList->isIdiomaticZeroInitializer(SemaRef.getLangOpts());
   bool HasDesignatedInit = false;
+  bool HasNonDesignatedInit = false;
 
   while (Index < IList->getNumInits()) {
 Expr *Init = IList->getInit(Index);
@@ -2013,6 +2014,10 @@
 
   HasDesignatedInit = true;
 
+  unsigned LastIdx =
+  Field != FieldEnd ? Field->getFieldIndex()
+: std::distance(RD->field_begin(), RD->field_end());
+
   // Handle this designated initializer. Field will be updated to
   // the next field that we'll be initializing.
   if (CheckDesignatedInitializer(Entity, IList, DIE, 0,
@@ -2032,6 +2037,20 @@
 }
   }
 
+  unsigned NextIdx =
+  Field != FieldEnd ? Field->getFieldIndex()
+: std::distance(RD->field_begin(), RD->field_end());
+  if (!VerifyOnly && SemaRef.getLangOpts().CPlusPlus2a) {
+if (LastIdx >= NextIdx) {
+  SemaRef.Diag(Init->getBeginLoc(), diag::ext_c20_designated_init)
+  << "out of order" << Init->getSourceRange();
+}
+if (HasNonDesignatedInit) {
+  SemaRef.Diag(Init->getBeginLoc(), diag::ext_c20_designated_init)
+  << "mixed" << Init->getSourceRange();
+}
+  }
+
   InitializedSomething = true;
 
   // Disable check for missing fields when designators are used.
@@ -2045,6 +2064,13 @@
   break;
 }
 
+HasNonDesignatedInit = true;
+
+if (!VerifyOnly && HasDesignatedInit && SemaRef.getLangOpts().CPlusPlus2a) {
+  SemaRef.Diag(Init->getBeginLoc(), diag::ext_c20_designated_init)
+  << "mixed" << Init->getSourceRange();
+}
+
 // We've already initialized a member of a union. We're done.
 if (InitializedSomething && DeclType->isUnionType())
   break;
@@ -2980,6 +3006,7 @@
   bool Invalid = false;
   SmallVector Designators;
   SmallVector InitExpressions;
+  bool HasArrayDesignator = false;
 
   // Build designators and check array designator expressions.
   for (unsigned Idx = 0; Idx < Desig.getNumDesignators(); ++Idx) {
@@ -3003,6 +3030,7 @@
 D.getRBracketLoc()));
 InitExpressions.push_back(Index);
   }
+  HasArrayDesignator = true;
   break;
 }
 
@@ -3046,6 +3074,7 @@
   InitExpressions.push_back(EndIndex);
 }
   }
+  HasArrayDesignator = true;
   break;
 }
 }
@@ -3063,9 +3092,19 @@
  InitExpressions, Loc, GNUSyntax,
  Init.getAs());
 
-  if (!getLangOpts().C99)
+  if (!getLangOpts().C99 && !getLangOpts().CPlusPlus2a) {
 Diag(DIE->getBeginLoc(), diag::ext_designated_init)
 << DIE->getSourceRange();
+  } else if (getLangOpts().CPlusPlus2a) {
+if (Desig.getNumDesignators() > 1) {
+  Diag(DIE->getBeginLoc(), diag::ext_c20_designated_init)
+  << "nested" << DIE->getSourceRange();
+}
+if (HasArrayDesignator) {
+  Diag(DIE->getBeginLoc(), diag::ext_c20_designated_init)
+  << "array" << DIE->getSourceRange();
+}
+  }
 
   return DIE;
 }
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===

[PATCH] D59754: [Sema] Add c++2a designated initializer warnings

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 195610.
hintonda added a comment.

- Removed auto and added specific warnings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59754

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaInit.cpp
  clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp

Index: clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
===
--- clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
+++ clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++2a %s -verify
+// RUN: %clang_cc1 -std=c++2a %s -verify -pedantic
 
 namespace class_with_ctor {
   struct A { // expected-note 6{{candidate}}
@@ -21,3 +21,14 @@
   C c1 = {{}, {}}; // ok, call default ctor twice
   C c2 = {{1, 2}, {3, 4}}; // expected-error 2{{no matching constructor}}
 }
+
+namespace designator {
+struct A { int x, y; };
+struct B { A a; };
+
+A a1 = {.y = 1, .x = 2}; // expected-warning {{C99 out of order designated initializers are a C++20 extention}}
+int arr[3] = {[1] = 5}; // expected-warning {{C99 array designated initializers are a C++20 extention}}
+B b = {.a.x = 0}; // expected-warning {{C99 nested designated initializers are a C++20 extention}}
+A a2 = {.x = 1, 2}; // expected-warning {{C99 mixed designated initializers are a C++20 extention}}
+A a3 = {1, .y = 2}; // expected-warning {{C99 mixed designated initializers are a C++20 extention}}
+}
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -1999,6 +1999,7 @@
   bool CheckForMissingFields =
 !IList->isIdiomaticZeroInitializer(SemaRef.getLangOpts());
   bool HasDesignatedInit = false;
+  bool HasNonDesignatedInit = false;
 
   while (Index < IList->getNumInits()) {
 Expr *Init = IList->getInit(Index);
@@ -2013,6 +2014,10 @@
 
   HasDesignatedInit = true;
 
+  unsigned LastIdx =
+  Field != FieldEnd ? Field->getFieldIndex()
+: std::distance(RD->field_begin(), RD->field_end());
+
   // Handle this designated initializer. Field will be updated to
   // the next field that we'll be initializing.
   if (CheckDesignatedInitializer(Entity, IList, DIE, 0,
@@ -2032,6 +2037,20 @@
 }
   }
 
+  unsigned NextIdx =
+  Field != FieldEnd ? Field->getFieldIndex()
+: std::distance(RD->field_begin(), RD->field_end());
+  if (!VerifyOnly && SemaRef.getLangOpts().CPlusPlus2a) {
+if (LastIdx >= NextIdx) {
+  SemaRef.Diag(Init->getBeginLoc(), diag::ext_c20_designated_init)
+  << "out of order" << Init->getSourceRange();
+}
+if (HasNonDesignatedInit) {
+  SemaRef.Diag(Init->getBeginLoc(), diag::ext_c20_designated_init)
+  << "mixed" << Init->getSourceRange();
+}
+  }
+
   InitializedSomething = true;
 
   // Disable check for missing fields when designators are used.
@@ -2045,6 +2064,13 @@
   break;
 }
 
+HasNonDesignatedInit = true;
+
+if (!VerifyOnly && HasDesignatedInit && SemaRef.getLangOpts().CPlusPlus2a) {
+  SemaRef.Diag(Init->getBeginLoc(), diag::ext_c20_designated_init)
+  << "mixed" << Init->getSourceRange();
+}
+
 // We've already initialized a member of a union. We're done.
 if (InitializedSomething && DeclType->isUnionType())
   break;
@@ -2980,6 +3006,7 @@
   bool Invalid = false;
   SmallVector Designators;
   SmallVector InitExpressions;
+  bool HasArrayDesignator = false;
 
   // Build designators and check array designator expressions.
   for (unsigned Idx = 0; Idx < Desig.getNumDesignators(); ++Idx) {
@@ -3003,6 +3030,7 @@
 D.getRBracketLoc()));
 InitExpressions.push_back(Index);
   }
+  HasArrayDesignator = true;
   break;
 }
 
@@ -3046,6 +3074,7 @@
   InitExpressions.push_back(EndIndex);
 }
   }
+  HasArrayDesignator = true;
   break;
 }
 }
@@ -3063,9 +3092,19 @@
  InitExpressions, Loc, GNUSyntax,
  Init.getAs());
 
-  if (!getLangOpts().C99)
+  if (!getLangOpts().C99 && !getLangOpts().CPlusPlus2a) {
 Diag(DIE->getBeginLoc(), diag::ext_designated_init)
 << DIE->getSourceRange();
+  } else if (getLangOpts().CPlusPlus2a) {
+if (Desig.getNumDesignators() > 1) {
+  Diag(DIE->getBeginLoc(), diag::ext_c20_designated_init)
+  << "nested" << DIE->getSourceRange();
+}
+if (HasArrayDesignator) {
+  Diag(DIE->getBeginLoc(), diag::ext_c20_designated_init)
+  << "array" << DIE->getSourceRange();
+}
+  }
 
   return DIE;
 }
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_check'

[Don Hinton via Phabricator via cfe-commits]

hintonda marked 2 inline comments as done.
hintonda added inline comments.



Comment at: clang-tools-extra/clang-tidy/add_new_check.py:381
+  # Don't allow 'clang' or 'llvm' namespaces
+  if module == 'llvm' or module == 'clang':
+namespace = module + '_check'

alexfh wrote:
> We don't have the `clang` module. No need to check for it here. If we ever 
> add one (which I doubt), we can modify this script again. But for now let's 
> remove this to avoid confusion.
No problem.  Or we could outlaw it...



Comment at: clang-tools-extra/clang-tidy/rename_check.py:268
+  if old_module != new_module or new_module == 'llvm' or new_module == 'clang':
+if new_module == 'llvm' or new_module -- 'clang':
+  new_namespace = new_module + '_check'

alexfh wrote:
> `--`?
> 
> Anyways, we don't have (or plan to have) a module named `clang`.
Thanks for catching that...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60629



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

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_check'

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 195581.
hintonda added a comment.

- Remove clang module check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60629

Files:
  clang-tools-extra/clang-tidy/add_new_check.py
  clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.cpp
  clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.h
  clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp
  clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.h
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.cpp
  clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.h
  clang-tools-extra/clang-tidy/rename_check.py
  clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
@@ -3,7 +3,7 @@
 #include "llvm/IncludeOrderCheck.h"
 #include "gtest/gtest.h"
 
-using namespace clang::tidy::llvm;
+using namespace clang::tidy::llvm_check;
 
 namespace clang {
 namespace tidy {
Index: clang-tools-extra/clang-tidy/rename_check.py
===
--- clang-tools-extra/clang-tidy/rename_check.py
+++ clang-tools-extra/clang-tidy/rename_check.py
@@ -14,6 +14,18 @@
 import re
 
 
+def replaceInFileRegex(fileName, sFrom, sTo):
+  if sFrom == sTo:
+return
+  txt = None
+  with open(fileName, "r") as f:
+txt = f.read()
+
+  txt = re.sub(sFrom, sTo, txt)
+  print("Replacing '%s' -> '%s' in '%s'..." % (sFrom, sTo, fileName))
+  with open(fileName, "w") as f:
+f.write(txt)
+
 def replaceInFile(fileName, sFrom, sTo):
   if sFrom == sTo:
 return
@@ -203,6 +215,8 @@
   clang_tidy_path = os.path.dirname(__file__)
 
   header_guard_variants = [
+  (args.old_check_name.replace('-', '_') + '_Check').upper(),
+  (old_module + '_' + check_name_camel).upper(),
   (old_module + '_' + new_check_name_camel).upper(),
   args.old_check_name.replace('-', '_').upper()]
   header_guard_new = (new_module + '_' + new_check_name_camel).upper()
@@ -210,18 +224,19 @@
   old_module_path = os.path.join(clang_tidy_path, old_module)
   new_module_path = os.path.join(clang_tidy_path, new_module)
 
-  # Remove the check from the old module.
-  cmake_lists = os.path.join(old_module_path, 'CMakeLists.txt')
-  check_found = deleteMatchingLines(cmake_lists, '\\b' + check_name_camel)
-  if not check_found:
-print("Check name '%s' not found in %s. Exiting." %
+  if (args.old_check_name != args.new_check_name):
+# Remove the check from the old module.
+cmake_lists = os.path.join(old_module_path, 'CMakeLists.txt')
+check_found = deleteMatchingLines(cmake_lists, '\\b' + check_name_camel)
+if not check_found:
+  print("Check name '%s' not found in %s. Exiting." %
 (check_name_camel, cmake_lists))
-return 1
+  return 1
 
-  modulecpp = filter(
-  lambda p: p.lower() == old_module.lower() + 'tidymodule.cpp',
-  os.listdir(old_module_path))[0]
-  deleteMatchingLines(os.path.join(old_module_path, modulecpp),
+modulecpp = filter(
+lambda p: p.lower() == old_module.lower() + 'tidymodule.cpp',
+os.listdir(old_module_path))[0]
+deleteMatchingLines(os.path.join(old_module_path, modulecpp),
   '\\b' + check_name_camel + '|\\b' + args.old_check_name)
 
   for filename in getListOfFiles(clang_tidy_path):
@@ -249,14 +264,21 @@
   new_module + '/' + new_check_name_camel)
 replaceInFile(filename, check_name_camel, new_check_name_camel)
 
-  if old_module != new_module:
+  if old_module != new_module or new_module == 'llvm':
+if new_module == 'llvm':
+  new_namespace = new_module + '_check'
+else:
+  new_namespace = new_module
 check_implementation_files = glob.glob(
 os.path.join(old_module_path, new_check_name_camel + '*'))
 for filename in check_implementation_files:
   # Move check implementation to the directory of the new module.
   filename = fileRename(filename, old_module_path, new_module_path)
-  replaceInFile(filename, 'namespace ' + old_module,
-'namespace ' + new_module)
+  replaceInFileRegex(filename, 'namespace ' + old_module + '[^ \n]*',
+ 'namespace ' + new_namespace)
+
+  if (args.old_check_name == args.new_check_name):
+return
 
   # Add check to the new module.
   adapt_cmake(new_module_path, new_check_name_camel)
Index: clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.h
===
--- clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.h
+++ clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.h
@@ -6,14 +6,14 @@
 //
 //===

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

[Don Hinton via Phabricator via cfe-commits]

hintonda marked an inline comment as done.
hintonda added inline comments.



Comment at: clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp:27-28
 CheckFactories.registerCheck("llvm-include-order");
+CheckFactories.registerCheck(
+"llvm-prefer-isa-or-dyn-cast-in-conditionals");
 CheckFactories.registerCheck(

hintonda wrote:
> aaron.ballman wrote:
> > hintonda wrote:
> > > aaron.ballman wrote:
> > > > Please keep this list sorted alphabetically.
> > > I think this is where add_new_check.py put it, but I'll alphabetize.
> > Huh, I thought that script kept everything alphabetized? That may be worth 
> > investigating (don't feel obligated though).
> I'm sure it's something I did inadvertently, but can't really remember the 
> details. 
Looks like rename_check.py definitely does not alphabetize.  I'll try to 
address than in another patch.  Thanks for pointing this out. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

[Don Hinton via Phabricator via cfe-commits]

hintonda marked an inline comment as done.
hintonda added inline comments.



Comment at: clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp:27-28
 CheckFactories.registerCheck("llvm-include-order");
+CheckFactories.registerCheck(
+"llvm-prefer-isa-or-dyn-cast-in-conditionals");
 CheckFactories.registerCheck(

aaron.ballman wrote:
> hintonda wrote:
> > aaron.ballman wrote:
> > > Please keep this list sorted alphabetically.
> > I think this is where add_new_check.py put it, but I'll alphabetize.
> Huh, I thought that script kept everything alphabetized? That may be worth 
> investigating (don't feel obligated though).
I'm sure it's something I did inadvertently, but can't really remember the 
details. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_check'

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 195278.
hintonda added a comment.

- Change `llvm_checker` to `llvm_check`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60629

Files:
  clang-tools-extra/clang-tidy/add_new_check.py
  clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.cpp
  clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.h
  clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp
  clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.h
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.cpp
  clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.h
  clang-tools-extra/clang-tidy/rename_check.py
  clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
@@ -3,7 +3,7 @@
 #include "llvm/IncludeOrderCheck.h"
 #include "gtest/gtest.h"
 
-using namespace clang::tidy::llvm;
+using namespace clang::tidy::llvm_check;
 
 namespace clang {
 namespace tidy {
Index: clang-tools-extra/clang-tidy/rename_check.py
===
--- clang-tools-extra/clang-tidy/rename_check.py
+++ clang-tools-extra/clang-tidy/rename_check.py
@@ -14,6 +14,18 @@
 import re
 
 
+def replaceInFileRegex(fileName, sFrom, sTo):
+  if sFrom == sTo:
+return
+  txt = None
+  with open(fileName, "r") as f:
+txt = f.read()
+
+  txt = re.sub(sFrom, sTo, txt)
+  print("Replacing '%s' -> '%s' in '%s'..." % (sFrom, sTo, fileName))
+  with open(fileName, "w") as f:
+f.write(txt)
+
 def replaceInFile(fileName, sFrom, sTo):
   if sFrom == sTo:
 return
@@ -203,6 +215,8 @@
   clang_tidy_path = os.path.dirname(__file__)
 
   header_guard_variants = [
+  (args.old_check_name.replace('-', '_') + '_Check').upper(),
+  (old_module + '_' + check_name_camel).upper(),
   (old_module + '_' + new_check_name_camel).upper(),
   args.old_check_name.replace('-', '_').upper()]
   header_guard_new = (new_module + '_' + new_check_name_camel).upper()
@@ -210,18 +224,19 @@
   old_module_path = os.path.join(clang_tidy_path, old_module)
   new_module_path = os.path.join(clang_tidy_path, new_module)
 
-  # Remove the check from the old module.
-  cmake_lists = os.path.join(old_module_path, 'CMakeLists.txt')
-  check_found = deleteMatchingLines(cmake_lists, '\\b' + check_name_camel)
-  if not check_found:
-print("Check name '%s' not found in %s. Exiting." %
+  if (args.old_check_name != args.new_check_name):
+# Remove the check from the old module.
+cmake_lists = os.path.join(old_module_path, 'CMakeLists.txt')
+check_found = deleteMatchingLines(cmake_lists, '\\b' + check_name_camel)
+if not check_found:
+  print("Check name '%s' not found in %s. Exiting." %
 (check_name_camel, cmake_lists))
-return 1
+  return 1
 
-  modulecpp = filter(
-  lambda p: p.lower() == old_module.lower() + 'tidymodule.cpp',
-  os.listdir(old_module_path))[0]
-  deleteMatchingLines(os.path.join(old_module_path, modulecpp),
+modulecpp = filter(
+lambda p: p.lower() == old_module.lower() + 'tidymodule.cpp',
+os.listdir(old_module_path))[0]
+deleteMatchingLines(os.path.join(old_module_path, modulecpp),
   '\\b' + check_name_camel + '|\\b' + args.old_check_name)
 
   for filename in getListOfFiles(clang_tidy_path):
@@ -249,14 +264,21 @@
   new_module + '/' + new_check_name_camel)
 replaceInFile(filename, check_name_camel, new_check_name_camel)
 
-  if old_module != new_module:
+  if old_module != new_module or new_module == 'llvm' or new_module == 'clang':
+if new_module == 'llvm' or new_module -- 'clang':
+  new_namespace = new_module + '_check'
+else:
+  new_namespace = new_module
 check_implementation_files = glob.glob(
 os.path.join(old_module_path, new_check_name_camel + '*'))
 for filename in check_implementation_files:
   # Move check implementation to the directory of the new module.
   filename = fileRename(filename, old_module_path, new_module_path)
-  replaceInFile(filename, 'namespace ' + old_module,
-'namespace ' + new_module)
+  replaceInFileRegex(filename, 'namespace ' + old_module + '[^ \n]*',
+ 'namespace ' + new_namespace)
+
+  if (args.old_check_name == args.new_check_name):
+return
 
   # Add check to the new module.
   adapt_cmake(new_module_path, new_check_name_camel)
Index: clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.h
===
--- clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.h
+++ clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 195270.
hintonda added a comment.

- Alphabetize registration calls.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802

Files:
  clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp
  clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/llvm-prefer-isa-or-dyn-cast-in-conditionals.rst
  
clang-tools-extra/test/clang-tidy/llvm-prefer-isa-or-dyn-cast-in-conditionals.cpp

Index: clang-tools-extra/test/clang-tidy/llvm-prefer-isa-or-dyn-cast-in-conditionals.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/llvm-prefer-isa-or-dyn-cast-in-conditionals.cpp
@@ -0,0 +1,132 @@
+// RUN: %check_clang_tidy %s llvm-prefer-isa-or-dyn-cast-in-conditionals %t
+
+struct X;
+struct Y;
+struct Z {
+  int foo();
+  X *bar();
+  X *cast(Y*);
+  bool baz(Y*);
+};
+
+template 
+bool isa(Y *);
+template 
+X *cast(Y *);
+template 
+X *dyn_cast(Y *);
+template 
+X *dyn_cast_or_null(Y *);
+
+bool foo(Y *y, Z *z) {
+  if (auto x = cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: cast<> in conditional will assert rather than return a null pointer [llvm-prefer-isa-or-dyn-cast-in-conditionals]
+  // CHECK-FIXES: if (auto x = dyn_cast(y))
+
+  while (auto x = cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:19: warning: cast<> in conditional
+  // CHECK-FIXES: while (auto x = dyn_cast(y))
+
+  if (cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: cast<> in conditional
+  // CHECK-FIXES: if (isa(y))
+
+  while (cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning: cast<> in conditional
+  // CHECK-FIXES: while (isa(y))
+
+  do {
+break;
+  } while (cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: cast<> in conditional
+  // CHECK-FIXES: while (isa(y));
+
+  if (dyn_cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: return value from dyn_cast<> not used [llvm-prefer-isa-or-dyn-cast-in-conditionals]
+  // CHECK-FIXES: if (isa(y))
+
+  while (dyn_cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning: return value from dyn_cast<> not used
+  // CHECK-FIXES: while (isa(y))
+
+  do {
+break;
+  } while (dyn_cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: return value from dyn_cast<> not used
+  // CHECK-FIXES: while (isa(y));
+
+  if (y && isa(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: isa_and_nonnull<> is preferred over an explicit test for null followed by calling isa<> [llvm-prefer-isa-or-dyn-cast-in-conditionals]
+  // CHECK-FIXES: if (isa_and_nonnull(y))
+
+  if (z->bar() && isa(z->bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  isa_and_nonnull<> is preferred
+  // CHECK-FIXES: if (isa_and_nonnull(z->bar()))
+
+  if (z->bar() && cast(z->bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: isa_and_nonnull<> is preferred
+  // CHECK-FIXES: if (isa_and_nonnull(z->bar()))
+
+  if (z->bar() && dyn_cast(z->bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: isa_and_nonnull<> is preferred
+  // CHECK-FIXES: if (isa_and_nonnull(z->bar()))
+
+  if (z->bar() && dyn_cast_or_null(z->bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: isa_and_nonnull<> is preferred
+  // CHECK-FIXES: if (isa_and_nonnull(z->bar()))
+
+  bool b = z->bar() && cast(z->bar());
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: isa_and_nonnull<> is preferred
+  // CHECK-FIXES: bool b = isa_and_nonnull(z->bar());
+
+  // These don't trigger a warning.
+  if (auto x = cast(y)->foo())
+return true;
+  if (auto x = z->cast(y))
+return true;
+  while (auto x = cast(y)->foo())
+break;
+  if (cast(y)->foo())
+return true;
+  if (z->cast(y))
+return true;
+  while (cast(y)->foo())
+break;
+  if (y && cast(z->bar()))
+return true;
+  if (z && cast(y)->foo())
+return true;
+  bool b2 = y && cast(z);
+  if(z->cast(y))
+return true;
+  if (z->baz(cast(z)))
+return true;
+
+#define CAST(T, Obj) cast(Obj)
+#define AUTO_VAR_CAST(X, Y, Z) auto X = cast(Z)
+#define ISA(T, Obj) isa(Obj)
+#define ISA_OR_NULL(T, Obj) Obj &&isa(Obj)
+
+  // Macros don't trigger warning.
+  if (auto x = CAST(X, y))
+return true;
+  if (AUTO_VAR_CAST(x, X, z))
+return true;
+  if (z->bar() && ISA(Y, z->bar()))
+return true;
+  if (ISA_OR_NULL(Y, z->bar()))
+return true;
+
+  return false;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/llvm-prefer-isa-or-dyn-cast-in-conditionals.rst
==

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_check'

[Don Hinton via Phabricator via cfe-commits]

hintonda marked an inline comment as done.
hintonda added inline comments.



Comment at: clang-tools-extra/clang-tidy/add_new_check.py:382
+  if module == 'llvm' or module == 'clang':
+namespace = module + '_checker'
+  else:

aaron.ballman wrote:
> I thought we were going with `llvm_check`?
Just typo on my part.  I the options mentioned before where things like 
`llvm_project` or `llvm_proj`, etc.  I just settled on `llvm_check` because it 
sounded better -- then actually typed `checker`.  

I'll fix it this afternoon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60629



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

[Don Hinton via Phabricator via cfe-commits]

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

In D59802#1467363 , @aaron.ballman 
wrote:

> LGTM aside from a nit and the ultimate name for the `isa_and_nonnull<>` API.


Thanks.  I'll ping the list again at the end of the week, then make whatever 
change is necessary.

Btw, the current iteration replaces all `v && isa(v)` instances with 
`isa_and_nonnull(v)`.  Is that still okay, or should it just do method calls?




Comment at: clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp:27-28
 CheckFactories.registerCheck("llvm-include-order");
+CheckFactories.registerCheck(
+"llvm-prefer-isa-or-dyn-cast-in-conditionals");
 CheckFactories.registerCheck(

aaron.ballman wrote:
> Please keep this list sorted alphabetically.
I think this is where add_new_check.py put it, but I'll alphabetize.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 195218.
hintonda added a comment.

- Add new test back.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746

Files:
  clang/lib/Tooling/CommonOptionsParser.cpp
  llvm/docs/CommandLine.rst
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp
  llvm/test/Support/check-default-options.txt
  llvm/tools/llvm-opt-report/OptReport.cpp
  llvm/unittests/Support/CommandLineTest.cpp

Index: llvm/unittests/Support/CommandLineTest.cpp
===
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -620,6 +620,68 @@
   }
 }
 
+TEST(CommandLineTest, DefaultOptions) {
+  cl::ResetCommandLineParser();
+
+  StackOption Bar("bar", cl::sub(*cl::AllSubCommands),
+   cl::DefaultOption);
+  StackOption Bar_Alias(
+  "b", cl::desc("Alias for -bar"), cl::aliasopt(Bar), cl::DefaultOption);
+
+  StackOption Foo("foo", cl::init(false), cl::sub(*cl::AllSubCommands),
+cl::DefaultOption);
+  StackOption Foo_Alias("f", cl::desc("Alias for -foo"),
+ cl::aliasopt(Foo), cl::DefaultOption);
+
+  StackSubCommand SC1("sc1", "First Subcommand");
+  // Override "-b" and change type in sc1 SubCommand.
+  StackOption SC1_B("b", cl::sub(SC1), cl::init(false));
+  StackSubCommand SC2("sc2", "Second subcommand");
+  // Override "-foo" and change type in sc2 SubCommand.  Note that this does not
+  // affect "-f" alias, which continues to work correctly.
+  StackOption SC2_Foo("foo", cl::sub(SC2));
+
+  const char *args0[] = {"prog", "-b", "args0 bar string", "-f"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args0) / sizeof(char *), args0,
+  StringRef(), &llvm::nulls()));
+  EXPECT_TRUE(Bar == "args0 bar string");
+  EXPECT_TRUE(Foo);
+  EXPECT_FALSE(SC1_B);
+  EXPECT_TRUE(SC2_Foo.empty());
+
+  cl::ResetAllOptionOccurrences();
+
+  const char *args1[] = {"prog", "sc1", "-b", "-bar", "args1 bar string", "-f"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args1) / sizeof(char *), args1,
+  StringRef(), &llvm::nulls()));
+  EXPECT_TRUE(Bar == "args1 bar string");
+  EXPECT_TRUE(Foo);
+  EXPECT_TRUE(SC1_B);
+  EXPECT_TRUE(SC2_Foo.empty());
+  for (auto *S : cl::getRegisteredSubcommands()) {
+if (*S) {
+  EXPECT_EQ("sc1", S->getName());
+}
+  }
+
+  cl::ResetAllOptionOccurrences();
+
+  const char *args2[] = {"prog", "sc2", "-b", "args2 bar string",
+ "-f", "-foo", "foo string"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args2) / sizeof(char *), args2,
+  StringRef(), &llvm::nulls()));
+  EXPECT_TRUE(Bar == "args2 bar string");
+  EXPECT_TRUE(Foo);
+  EXPECT_FALSE(SC1_B);
+  EXPECT_TRUE(SC2_Foo == "foo string");
+  for (auto *S : cl::getRegisteredSubcommands()) {
+if (*S) {
+  EXPECT_EQ("sc2", S->getName());
+}
+  }
+  cl::ResetCommandLineParser();
+}
+
 TEST(CommandLineTest, ArgumentLimit) {
   std::string args(32 * 4096, 'a');
   EXPECT_FALSE(llvm::sys::commandLineFitsWithinSystemLimits("cl", args.data()));
Index: llvm/tools/llvm-opt-report/OptReport.cpp
===
--- llvm/tools/llvm-opt-report/OptReport.cpp
+++ llvm/tools/llvm-opt-report/OptReport.cpp
@@ -36,8 +36,6 @@
 using namespace llvm;
 using namespace llvm::yaml;
 
-static cl::opt Help("h", cl::desc("Alias for -help"), cl::Hidden);
-
 // Mark all our options with this category, everything else (except for -version
 // and -help) will be hidden.
 static cl::OptionCategory
@@ -440,11 +438,6 @@
   "A tool to generate an optimization report from YAML optimization"
   " record files.\n");
 
-  if (Help) {
-cl::PrintHelpMessage();
-return 0;
-  }
-
   LocationInfoTy LocationInfo;
   if (!readLocationInfo(LocationInfo))
 return 1;
Index: llvm/test/Support/check-default-options.txt
===
--- /dev/null
+++ llvm/test/Support/check-default-options.txt
@@ -0,0 +1,18 @@
+# RUN: llvm-objdump -help-hidden %t | FileCheck --check-prefix=CHECK-OBJDUMP %s
+# RUN: llvm-readobj -help-hidden %t | FileCheck --check-prefix=CHECK-READOBJ %s
+# RUN: llvm-tblgen -help-hidden %t | FileCheck --check-prefix=CHECK-TBLGEN %s
+# RUN: llvm-opt-report -help-hidden %t | FileCheck --check-prefix=CHECK-OPT-RPT %s
+# RUN: llvm-dwarfdump -help-hidden %t | FileCheck --check-prefix=CHECK-DWARF %s
+# RUN: llvm-dwarfdump -h %t | FileCheck --check-prefix=CHECK-DWARF-H %s
+
+
+# CHECK-OBJDUMP: -h  - Alias for --section-headers
+# CHECK-READOBJ: -h  - Alias for --file-headers
+# CHECK-TBLGEN:  -h  - Alias for -help
+# CHECK-OPT-RPT: -h  - Alias for -help
+# CHECK-DWARF:   -h  - A

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 195217.
hintonda added a comment.

- Original patch, r358337, that was reverted.
- Make sure to clear DefaultOptions on reset.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746

Files:
  clang/lib/Tooling/CommonOptionsParser.cpp
  llvm/docs/CommandLine.rst
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp
  llvm/tools/llvm-opt-report/OptReport.cpp
  llvm/unittests/Support/CommandLineTest.cpp

Index: llvm/unittests/Support/CommandLineTest.cpp
===
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -620,6 +620,68 @@
   }
 }
 
+TEST(CommandLineTest, DefaultOptions) {
+  cl::ResetCommandLineParser();
+
+  StackOption Bar("bar", cl::sub(*cl::AllSubCommands),
+   cl::DefaultOption);
+  StackOption Bar_Alias(
+  "b", cl::desc("Alias for -bar"), cl::aliasopt(Bar), cl::DefaultOption);
+
+  StackOption Foo("foo", cl::init(false), cl::sub(*cl::AllSubCommands),
+cl::DefaultOption);
+  StackOption Foo_Alias("f", cl::desc("Alias for -foo"),
+ cl::aliasopt(Foo), cl::DefaultOption);
+
+  StackSubCommand SC1("sc1", "First Subcommand");
+  // Override "-b" and change type in sc1 SubCommand.
+  StackOption SC1_B("b", cl::sub(SC1), cl::init(false));
+  StackSubCommand SC2("sc2", "Second subcommand");
+  // Override "-foo" and change type in sc2 SubCommand.  Note that this does not
+  // affect "-f" alias, which continues to work correctly.
+  StackOption SC2_Foo("foo", cl::sub(SC2));
+
+  const char *args0[] = {"prog", "-b", "args0 bar string", "-f"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args0) / sizeof(char *), args0,
+  StringRef(), &llvm::nulls()));
+  EXPECT_TRUE(Bar == "args0 bar string");
+  EXPECT_TRUE(Foo);
+  EXPECT_FALSE(SC1_B);
+  EXPECT_TRUE(SC2_Foo.empty());
+
+  cl::ResetAllOptionOccurrences();
+
+  const char *args1[] = {"prog", "sc1", "-b", "-bar", "args1 bar string", "-f"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args1) / sizeof(char *), args1,
+  StringRef(), &llvm::nulls()));
+  EXPECT_TRUE(Bar == "args1 bar string");
+  EXPECT_TRUE(Foo);
+  EXPECT_TRUE(SC1_B);
+  EXPECT_TRUE(SC2_Foo.empty());
+  for (auto *S : cl::getRegisteredSubcommands()) {
+if (*S) {
+  EXPECT_EQ("sc1", S->getName());
+}
+  }
+
+  cl::ResetAllOptionOccurrences();
+
+  const char *args2[] = {"prog", "sc2", "-b", "args2 bar string",
+ "-f", "-foo", "foo string"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args2) / sizeof(char *), args2,
+  StringRef(), &llvm::nulls()));
+  EXPECT_TRUE(Bar == "args2 bar string");
+  EXPECT_TRUE(Foo);
+  EXPECT_FALSE(SC1_B);
+  EXPECT_TRUE(SC2_Foo == "foo string");
+  for (auto *S : cl::getRegisteredSubcommands()) {
+if (*S) {
+  EXPECT_EQ("sc2", S->getName());
+}
+  }
+  cl::ResetCommandLineParser();
+}
+
 TEST(CommandLineTest, ArgumentLimit) {
   std::string args(32 * 4096, 'a');
   EXPECT_FALSE(llvm::sys::commandLineFitsWithinSystemLimits("cl", args.data()));
Index: llvm/tools/llvm-opt-report/OptReport.cpp
===
--- llvm/tools/llvm-opt-report/OptReport.cpp
+++ llvm/tools/llvm-opt-report/OptReport.cpp
@@ -36,8 +36,6 @@
 using namespace llvm;
 using namespace llvm::yaml;
 
-static cl::opt Help("h", cl::desc("Alias for -help"), cl::Hidden);
-
 // Mark all our options with this category, everything else (except for -version
 // and -help) will be hidden.
 static cl::OptionCategory
@@ -440,11 +438,6 @@
   "A tool to generate an optimization report from YAML optimization"
   " record files.\n");
 
-  if (Help) {
-cl::PrintHelpMessage();
-return 0;
-  }
-
   LocationInfoTy LocationInfo;
   if (!readLocationInfo(LocationInfo))
 return 1;
Index: llvm/lib/Support/CommandLine.cpp
===
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -98,6 +98,11 @@
   // This collects additional help to be printed.
   std::vector MoreHelp;
 
+  // This collects Options added with the cl::DefaultOption flag. Since they can
+  // be overridden, they are not added to the appropriate SubCommands until
+  // ParseCommandLineOptions actually runs.
+  SmallVector DefaultOptions;
+
   // This collects the different option categories that have been registered.
   SmallPtrSet RegisteredOptionCategories;
 
@@ -146,6 +151,11 @@
   void addOption(Option *O, SubCommand *SC) {
 bool HadErrors = false;
 if (O->hasArgStr()) {
+  // If it's a DefaultOption, check to make sure it isn't already there.
+  if (O->isDefa

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

[Don Hinton via Phabricator via cfe-commits]

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

Reopen to track fix after buildbot failure and revert -- r358414.

http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20190415/644037.html


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59746



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

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D59746#1465445 , @klimek wrote:

> lg


Thanks again for the help and the suggestion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



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

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_checker'

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D60629#1464945 , @Eugene.Zelenko 
wrote:

> Please use check for consistency with rest of Clang-tidy.


Thanks for taking a look — I’ll Fix that on the next upload.  So are you okay 
with llvm_check?  I sorta like it, but happy to use anything other than llvm, 
which caused the initial problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60629



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

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_checker'

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 194940.
hintonda added a comment.

- Remove leftover comments, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60629

Files:
  clang-tools-extra/clang-tidy/add_new_check.py
  clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.cpp
  clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.h
  clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp
  clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.h
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.cpp
  clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.h
  clang-tools-extra/clang-tidy/rename_check.py
  clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
@@ -3,7 +3,7 @@
 #include "llvm/IncludeOrderCheck.h"
 #include "gtest/gtest.h"
 
-using namespace clang::tidy::llvm;
+using namespace clang::tidy::llvm_checker;
 
 namespace clang {
 namespace tidy {
Index: clang-tools-extra/clang-tidy/rename_check.py
===
--- clang-tools-extra/clang-tidy/rename_check.py
+++ clang-tools-extra/clang-tidy/rename_check.py
@@ -14,6 +14,18 @@
 import re
 
 
+def replaceInFileRegex(fileName, sFrom, sTo):
+  if sFrom == sTo:
+return
+  txt = None
+  with open(fileName, "r") as f:
+txt = f.read()
+
+  txt = re.sub(sFrom, sTo, txt)
+  print("Replacing '%s' -> '%s' in '%s'..." % (sFrom, sTo, fileName))
+  with open(fileName, "w") as f:
+f.write(txt)
+
 def replaceInFile(fileName, sFrom, sTo):
   if sFrom == sTo:
 return
@@ -203,6 +215,8 @@
   clang_tidy_path = os.path.dirname(__file__)
 
   header_guard_variants = [
+  (args.old_check_name.replace('-', '_') + '_Check').upper(),
+  (old_module + '_' + check_name_camel).upper(),
   (old_module + '_' + new_check_name_camel).upper(),
   args.old_check_name.replace('-', '_').upper()]
   header_guard_new = (new_module + '_' + new_check_name_camel).upper()
@@ -210,18 +224,19 @@
   old_module_path = os.path.join(clang_tidy_path, old_module)
   new_module_path = os.path.join(clang_tidy_path, new_module)
 
-  # Remove the check from the old module.
-  cmake_lists = os.path.join(old_module_path, 'CMakeLists.txt')
-  check_found = deleteMatchingLines(cmake_lists, '\\b' + check_name_camel)
-  if not check_found:
-print("Check name '%s' not found in %s. Exiting." %
+  if (args.old_check_name != args.new_check_name):
+# Remove the check from the old module.
+cmake_lists = os.path.join(old_module_path, 'CMakeLists.txt')
+check_found = deleteMatchingLines(cmake_lists, '\\b' + check_name_camel)
+if not check_found:
+  print("Check name '%s' not found in %s. Exiting." %
 (check_name_camel, cmake_lists))
-return 1
+  return 1
 
-  modulecpp = filter(
-  lambda p: p.lower() == old_module.lower() + 'tidymodule.cpp',
-  os.listdir(old_module_path))[0]
-  deleteMatchingLines(os.path.join(old_module_path, modulecpp),
+modulecpp = filter(
+lambda p: p.lower() == old_module.lower() + 'tidymodule.cpp',
+os.listdir(old_module_path))[0]
+deleteMatchingLines(os.path.join(old_module_path, modulecpp),
   '\\b' + check_name_camel + '|\\b' + args.old_check_name)
 
   for filename in getListOfFiles(clang_tidy_path):
@@ -249,14 +264,21 @@
   new_module + '/' + new_check_name_camel)
 replaceInFile(filename, check_name_camel, new_check_name_camel)
 
-  if old_module != new_module:
+  if old_module != new_module or new_module == 'llvm' or new_module == 'clang':
+if new_module == 'llvm':
+  new_namespace = new_module + '_checker'
+else:
+  new_namespace = new_module
 check_implementation_files = glob.glob(
 os.path.join(old_module_path, new_check_name_camel + '*'))
 for filename in check_implementation_files:
   # Move check implementation to the directory of the new module.
   filename = fileRename(filename, old_module_path, new_module_path)
-  replaceInFile(filename, 'namespace ' + old_module,
-'namespace ' + new_module)
+  replaceInFileRegex(filename, 'namespace ' + old_module + '[^ \n]*',
+ 'namespace ' + new_namespace)
+
+  if (args.old_check_name == args.new_check_name):
+return
 
   # Add check to the new module.
   adapt_cmake(new_module_path, new_check_name_camel)
Index: clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.h
===
--- clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.h
+++ clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.h
@@ -6,14 +6,14 @@
 //
 //==

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_checker'

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 194941.
hintonda added a comment.

- Add missing 'clang'.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60629

Files:
  clang-tools-extra/clang-tidy/add_new_check.py
  clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.cpp
  clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.h
  clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp
  clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.h
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.cpp
  clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.h
  clang-tools-extra/clang-tidy/rename_check.py
  clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
@@ -3,7 +3,7 @@
 #include "llvm/IncludeOrderCheck.h"
 #include "gtest/gtest.h"
 
-using namespace clang::tidy::llvm;
+using namespace clang::tidy::llvm_checker;
 
 namespace clang {
 namespace tidy {
Index: clang-tools-extra/clang-tidy/rename_check.py
===
--- clang-tools-extra/clang-tidy/rename_check.py
+++ clang-tools-extra/clang-tidy/rename_check.py
@@ -14,6 +14,18 @@
 import re
 
 
+def replaceInFileRegex(fileName, sFrom, sTo):
+  if sFrom == sTo:
+return
+  txt = None
+  with open(fileName, "r") as f:
+txt = f.read()
+
+  txt = re.sub(sFrom, sTo, txt)
+  print("Replacing '%s' -> '%s' in '%s'..." % (sFrom, sTo, fileName))
+  with open(fileName, "w") as f:
+f.write(txt)
+
 def replaceInFile(fileName, sFrom, sTo):
   if sFrom == sTo:
 return
@@ -203,6 +215,8 @@
   clang_tidy_path = os.path.dirname(__file__)
 
   header_guard_variants = [
+  (args.old_check_name.replace('-', '_') + '_Check').upper(),
+  (old_module + '_' + check_name_camel).upper(),
   (old_module + '_' + new_check_name_camel).upper(),
   args.old_check_name.replace('-', '_').upper()]
   header_guard_new = (new_module + '_' + new_check_name_camel).upper()
@@ -210,18 +224,19 @@
   old_module_path = os.path.join(clang_tidy_path, old_module)
   new_module_path = os.path.join(clang_tidy_path, new_module)
 
-  # Remove the check from the old module.
-  cmake_lists = os.path.join(old_module_path, 'CMakeLists.txt')
-  check_found = deleteMatchingLines(cmake_lists, '\\b' + check_name_camel)
-  if not check_found:
-print("Check name '%s' not found in %s. Exiting." %
+  if (args.old_check_name != args.new_check_name):
+# Remove the check from the old module.
+cmake_lists = os.path.join(old_module_path, 'CMakeLists.txt')
+check_found = deleteMatchingLines(cmake_lists, '\\b' + check_name_camel)
+if not check_found:
+  print("Check name '%s' not found in %s. Exiting." %
 (check_name_camel, cmake_lists))
-return 1
+  return 1
 
-  modulecpp = filter(
-  lambda p: p.lower() == old_module.lower() + 'tidymodule.cpp',
-  os.listdir(old_module_path))[0]
-  deleteMatchingLines(os.path.join(old_module_path, modulecpp),
+modulecpp = filter(
+lambda p: p.lower() == old_module.lower() + 'tidymodule.cpp',
+os.listdir(old_module_path))[0]
+deleteMatchingLines(os.path.join(old_module_path, modulecpp),
   '\\b' + check_name_camel + '|\\b' + args.old_check_name)
 
   for filename in getListOfFiles(clang_tidy_path):
@@ -249,14 +264,21 @@
   new_module + '/' + new_check_name_camel)
 replaceInFile(filename, check_name_camel, new_check_name_camel)
 
-  if old_module != new_module:
+  if old_module != new_module or new_module == 'llvm' or new_module == 'clang':
+if new_module == 'llvm' or new_module -- 'clang':
+  new_namespace = new_module + '_checker'
+else:
+  new_namespace = new_module
 check_implementation_files = glob.glob(
 os.path.join(old_module_path, new_check_name_camel + '*'))
 for filename in check_implementation_files:
   # Move check implementation to the directory of the new module.
   filename = fileRename(filename, old_module_path, new_module_path)
-  replaceInFile(filename, 'namespace ' + old_module,
-'namespace ' + new_module)
+  replaceInFileRegex(filename, 'namespace ' + old_module + '[^ \n]*',
+ 'namespace ' + new_namespace)
+
+  if (args.old_check_name == args.new_check_name):
+return
 
   # Add check to the new module.
   adapt_cmake(new_module_path, new_check_name_camel)
Index: clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.h
===
--- clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.h
+++ clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.h
@@ -6,14 +6,

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_checker'

[Don Hinton via Phabricator via cfe-commits]

hintonda created this revision.
hintonda added reviewers: alexfh, aaron.ballman.
Herald added a subscriber: xazax.hun.
Herald added a project: clang.

Change the namespace for llvm checkers from 'llvm' to
'llvm_checker', and modify add_new_check.py and rename_check.py to
support the new namespace. Checker, file, and directory names remain
unchanged.

Used new version of rename_check.py to make the change in existing
llvm checkers, but had to fix LLVMTidyModule.cpp and
LLVMModuleTest.cpp by hand.

The changes to rename_check.py are idempotent, so if accidently
run multiple times, it won't do anything.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D60629

Files:
  clang-tools-extra/clang-tidy/add_new_check.py
  clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.cpp
  clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.h
  clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp
  clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.h
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.cpp
  clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.h
  clang-tools-extra/clang-tidy/rename_check.py
  clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
@@ -3,7 +3,7 @@
 #include "llvm/IncludeOrderCheck.h"
 #include "gtest/gtest.h"
 
-using namespace clang::tidy::llvm;
+using namespace clang::tidy::llvm_checker;
 
 namespace clang {
 namespace tidy {
Index: clang-tools-extra/clang-tidy/rename_check.py
===
--- clang-tools-extra/clang-tidy/rename_check.py
+++ clang-tools-extra/clang-tidy/rename_check.py
@@ -14,6 +14,23 @@
 import re
 
 
+def replaceInFileRegex(fileName, sFrom, sTo):
+  if sFrom == sTo:
+return
+  txt = None
+  with open(fileName, "r") as f:
+txt = f.read()
+
+  txt = re.sub(sFrom, sTo, txt)
+
+  #if sFrom not in txt:
+  #  return
+
+  #txt = txt.replace(sFrom, sTo)
+  print("Replacing '%s' -> '%s' in '%s'..." % (sFrom, sTo, fileName))
+  with open(fileName, "w") as f:
+f.write(txt)
+
 def replaceInFile(fileName, sFrom, sTo):
   if sFrom == sTo:
 return
@@ -203,25 +220,27 @@
   clang_tidy_path = os.path.dirname(__file__)
 
   header_guard_variants = [
-  (old_module + '_' + new_check_name_camel).upper(),
+  (args.old_check_name.replace('-', '_') + '_Check').upper(),
+  (old_module + '_' + check_name_camel).upper(),
   args.old_check_name.replace('-', '_').upper()]
   header_guard_new = (new_module + '_' + new_check_name_camel).upper()
 
   old_module_path = os.path.join(clang_tidy_path, old_module)
   new_module_path = os.path.join(clang_tidy_path, new_module)
 
-  # Remove the check from the old module.
-  cmake_lists = os.path.join(old_module_path, 'CMakeLists.txt')
-  check_found = deleteMatchingLines(cmake_lists, '\\b' + check_name_camel)
-  if not check_found:
-print("Check name '%s' not found in %s. Exiting." %
+  if (args.old_check_name != args.new_check_name):
+# Remove the check from the old module.
+cmake_lists = os.path.join(old_module_path, 'CMakeLists.txt')
+check_found = deleteMatchingLines(cmake_lists, '\\b' + check_name_camel)
+if not check_found:
+  print("Check name '%s' not found in %s. Exiting." %
 (check_name_camel, cmake_lists))
-return 1
+  return 1
 
-  modulecpp = filter(
-  lambda p: p.lower() == old_module.lower() + 'tidymodule.cpp',
-  os.listdir(old_module_path))[0]
-  deleteMatchingLines(os.path.join(old_module_path, modulecpp),
+modulecpp = filter(
+lambda p: p.lower() == old_module.lower() + 'tidymodule.cpp',
+os.listdir(old_module_path))[0]
+deleteMatchingLines(os.path.join(old_module_path, modulecpp),
   '\\b' + check_name_camel + '|\\b' + args.old_check_name)
 
   for filename in getListOfFiles(clang_tidy_path):
@@ -249,14 +268,23 @@
   new_module + '/' + new_check_name_camel)
 replaceInFile(filename, check_name_camel, new_check_name_camel)
 
-  if old_module != new_module:
+  if old_module != new_module or new_module == 'llvm' or new_module == 'clang':
+if new_module == 'llvm':
+  new_namespace = new_module + '_checker'
+else:
+  new_namespace = new_module
 check_implementation_files = glob.glob(
 os.path.join(old_module_path, new_check_name_camel + '*'))
 for filename in check_implementation_files:
   # Move check implementation to the directory of the new module.
   filename = fileRename(filename, old_module_path, new_module_path)
-  replaceInFile(filename, 'namespace ' + old_module,
-'namespace ' + new_module)
+  #replaceInFile(filename, 'namespace ' + old_module,
+

[PATCH] D60151: [clang-tidy] Rename llvm checkers to llvm-project

[Don Hinton via Phabricator via cfe-commits]

hintonda abandoned this revision.
hintonda added a comment.

Replaced by D60629 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60151



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

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 194899.
hintonda added a comment.

- Fix comments and add isDefaultOption per review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746

Files:
  clang/lib/Tooling/CommonOptionsParser.cpp
  llvm/docs/CommandLine.rst
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp
  llvm/test/Support/check-default-options.txt
  llvm/tools/llvm-opt-report/OptReport.cpp
  llvm/unittests/Support/CommandLineTest.cpp

Index: llvm/unittests/Support/CommandLineTest.cpp
===
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -620,6 +620,67 @@
   }
 }
 
+TEST(CommandLineTest, DefaultOptions) {
+  cl::ResetCommandLineParser();
+
+  StackOption Bar("bar", cl::sub(*cl::AllSubCommands),
+   cl::DefaultOption);
+  StackOption Bar_Alias(
+  "b", cl::desc("Alias for -bar"), cl::aliasopt(Bar), cl::DefaultOption);
+
+  StackOption Foo("foo", cl::init(false), cl::sub(*cl::AllSubCommands),
+cl::DefaultOption);
+  StackOption Foo_Alias("f", cl::desc("Alias for -foo"),
+ cl::aliasopt(Foo), cl::DefaultOption);
+
+  StackSubCommand SC1("sc1", "First Subcommand");
+  // Override "-b" and change type in sc1 SubCommand.
+  StackOption SC1_B("b", cl::sub(SC1), cl::init(false));
+  StackSubCommand SC2("sc2", "Second subcommand");
+  // Override "-foo" and change type in sc2 SubCommand.  Note that this does not
+  // affect "-f" alias, which continues to work correctly.
+  StackOption SC2_Foo("foo", cl::sub(SC2));
+
+  const char *args0[] = {"prog", "-b", "args0 bar string", "-f"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args0) / sizeof(char *), args0,
+  StringRef(), &llvm::nulls()));
+  EXPECT_TRUE(Bar == "args0 bar string");
+  EXPECT_TRUE(Foo);
+  EXPECT_FALSE(SC1_B);
+  EXPECT_TRUE(SC2_Foo.empty());
+
+  cl::ResetAllOptionOccurrences();
+
+  const char *args1[] = {"prog", "sc1", "-b", "-bar", "args1 bar string", "-f"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args1) / sizeof(char *), args1,
+  StringRef(), &llvm::nulls()));
+  EXPECT_TRUE(Bar == "args1 bar string");
+  EXPECT_TRUE(Foo);
+  EXPECT_TRUE(SC1_B);
+  EXPECT_TRUE(SC2_Foo.empty());
+  for (auto *S : cl::getRegisteredSubcommands()) {
+if (*S) {
+  EXPECT_EQ("sc1", S->getName());
+}
+  }
+
+  cl::ResetAllOptionOccurrences();
+
+  const char *args2[] = {"prog", "sc2", "-b", "args2 bar string",
+ "-f", "-foo", "foo string"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args2) / sizeof(char *), args2,
+  StringRef(), &llvm::nulls()));
+  EXPECT_TRUE(Bar == "args2 bar string");
+  EXPECT_TRUE(Foo);
+  EXPECT_FALSE(SC1_B);
+  EXPECT_TRUE(SC2_Foo == "foo string");
+  for (auto *S : cl::getRegisteredSubcommands()) {
+if (*S) {
+  EXPECT_EQ("sc2", S->getName());
+}
+  }
+}
+
 TEST(CommandLineTest, ArgumentLimit) {
   std::string args(32 * 4096, 'a');
   EXPECT_FALSE(llvm::sys::commandLineFitsWithinSystemLimits("cl", args.data()));
Index: llvm/tools/llvm-opt-report/OptReport.cpp
===
--- llvm/tools/llvm-opt-report/OptReport.cpp
+++ llvm/tools/llvm-opt-report/OptReport.cpp
@@ -36,8 +36,6 @@
 using namespace llvm;
 using namespace llvm::yaml;
 
-static cl::opt Help("h", cl::desc("Alias for -help"), cl::Hidden);
-
 // Mark all our options with this category, everything else (except for -version
 // and -help) will be hidden.
 static cl::OptionCategory
@@ -440,11 +438,6 @@
   "A tool to generate an optimization report from YAML optimization"
   " record files.\n");
 
-  if (Help) {
-cl::PrintHelpMessage();
-return 0;
-  }
-
   LocationInfoTy LocationInfo;
   if (!readLocationInfo(LocationInfo))
 return 1;
Index: llvm/test/Support/check-default-options.txt
===
--- /dev/null
+++ llvm/test/Support/check-default-options.txt
@@ -0,0 +1,18 @@
+# RUN: llvm-objdump -help-hidden %t | FileCheck --check-prefix=CHECK-OBJDUMP %s
+# RUN: llvm-readobj -help-hidden %t | FileCheck --check-prefix=CHECK-READOBJ %s
+# RUN: llvm-tblgen -help-hidden %t | FileCheck --check-prefix=CHECK-TBLGEN %s
+# RUN: llvm-opt-report -help-hidden %t | FileCheck --check-prefix=CHECK-OPT-RPT %s
+# RUN: llvm-dwarfdump -help-hidden %t | FileCheck --check-prefix=CHECK-DWARF %s
+# RUN: llvm-dwarfdump -h %t | FileCheck --check-prefix=CHECK-DWARF-H %s
+
+
+# CHECK-OBJDUMP: -h  - Alias for --section-headers
+# CHECK-READOBJ: -h  - Alias for --file-headers
+# CHECK-TBLGEN:  -h  - Alias for -help
+# CHECK-OPT-RPT: -h  - Alias for -help
+# CHECK-DWARF:   -

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 194782.
hintonda added a comment.

- Update isa_and_nonnull diagnostic message, and fix typo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802

Files:
  clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp
  clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/llvm-prefer-isa-or-dyn-cast-in-conditionals.rst
  
clang-tools-extra/test/clang-tidy/llvm-prefer-isa-or-dyn-cast-in-conditionals.cpp

Index: clang-tools-extra/test/clang-tidy/llvm-prefer-isa-or-dyn-cast-in-conditionals.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/llvm-prefer-isa-or-dyn-cast-in-conditionals.cpp
@@ -0,0 +1,132 @@
+// RUN: %check_clang_tidy %s llvm-prefer-isa-or-dyn-cast-in-conditionals %t
+
+struct X;
+struct Y;
+struct Z {
+  int foo();
+  X *bar();
+  X *cast(Y*);
+  bool baz(Y*);
+};
+
+template 
+bool isa(Y *);
+template 
+X *cast(Y *);
+template 
+X *dyn_cast(Y *);
+template 
+X *dyn_cast_or_null(Y *);
+
+bool foo(Y *y, Z *z) {
+  if (auto x = cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: cast<> in conditional will assert rather than return a null pointer [llvm-prefer-isa-or-dyn-cast-in-conditionals]
+  // CHECK-FIXES: if (auto x = dyn_cast(y))
+
+  while (auto x = cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:19: warning: cast<> in conditional
+  // CHECK-FIXES: while (auto x = dyn_cast(y))
+
+  if (cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: cast<> in conditional
+  // CHECK-FIXES: if (isa(y))
+
+  while (cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning: cast<> in conditional
+  // CHECK-FIXES: while (isa(y))
+
+  do {
+break;
+  } while (cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: cast<> in conditional
+  // CHECK-FIXES: while (isa(y));
+
+  if (dyn_cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: return value from dyn_cast<> not used [llvm-prefer-isa-or-dyn-cast-in-conditionals]
+  // CHECK-FIXES: if (isa(y))
+
+  while (dyn_cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning: return value from dyn_cast<> not used
+  // CHECK-FIXES: while (isa(y))
+
+  do {
+break;
+  } while (dyn_cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: return value from dyn_cast<> not used
+  // CHECK-FIXES: while (isa(y));
+
+  if (y && isa(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: isa_and_nonnull<> is preferred over an explicit test for null followed by calling isa<> [llvm-prefer-isa-or-dyn-cast-in-conditionals]
+  // CHECK-FIXES: if (isa_and_nonnull(y))
+
+  if (z->bar() && isa(z->bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  isa_and_nonnull<> is preferred
+  // CHECK-FIXES: if (isa_and_nonnull(z->bar()))
+
+  if (z->bar() && cast(z->bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: isa_and_nonnull<> is preferred
+  // CHECK-FIXES: if (isa_and_nonnull(z->bar()))
+
+  if (z->bar() && dyn_cast(z->bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: isa_and_nonnull<> is preferred
+  // CHECK-FIXES: if (isa_and_nonnull(z->bar()))
+
+  if (z->bar() && dyn_cast_or_null(z->bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: isa_and_nonnull<> is preferred
+  // CHECK-FIXES: if (isa_and_nonnull(z->bar()))
+
+  bool b = z->bar() && cast(z->bar());
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: isa_and_nonnull<> is preferred
+  // CHECK-FIXES: bool b = isa_and_nonnull(z->bar());
+
+  // These don't trigger a warning.
+  if (auto x = cast(y)->foo())
+return true;
+  if (auto x = z->cast(y))
+return true;
+  while (auto x = cast(y)->foo())
+break;
+  if (cast(y)->foo())
+return true;
+  if (z->cast(y))
+return true;
+  while (cast(y)->foo())
+break;
+  if (y && cast(z->bar()))
+return true;
+  if (z && cast(y)->foo())
+return true;
+  bool b2 = y && cast(z);
+  if(z->cast(y))
+return true;
+  if (z->baz(cast(z)))
+return true;
+
+#define CAST(T, Obj) cast(Obj)
+#define AUTO_VAR_CAST(X, Y, Z) auto X = cast(Z)
+#define ISA(T, Obj) isa(Obj)
+#define ISA_OR_NULL(T, Obj) Obj &&isa(Obj)
+
+  // Macros don't trigger warning.
+  if (auto x = CAST(X, y))
+return true;
+  if (AUTO_VAR_CAST(x, X, z))
+return true;
+  if (z->bar() && ISA(Y, z->bar()))
+return true;
+  if (ISA_OR_NULL(Y, z->bar()))
+return true;
+
+  return false;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/llvm-prefer-isa-or-dyn-cast-in-conditionals.rst
=

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

[Don Hinton via Phabricator via cfe-commits]

hintonda marked an inline comment as done.
hintonda added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.h:37-39
+///  if (var && isa(var)) {}
+///  // is replaced by:
+///  if (dyn_cast_or_null(var.foo())) {}

aaron.ballman wrote:
> I think this comment is now out of date.
Thanks, I'll fix that.  

I'll give the RFC a few more days, but not sure it's been definitive.  A few no 
votes, but there are always some of those.  I sorta like `isa_nonnull` for 
brevity, so I'll mention that why I follow-up on that thread.  Btw, the 
`isa_and_nonnull` has been in for a week: r357761, and nothing on that 
thread leads me to believe I should revert it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D60151: [clang-tidy] Rename llvm checkers to llvm-project

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D60151#1463484 , @MyDeveloperDay 
wrote:

> >> I suppose we could keep the names and directory structure and just change 
> >> the namespace.  That would just be a special case in the scripts.  Haven't 
> >> looked into it yet, but will do so as soon as I can.
> > 
> > Isn't that matching done on strings? I.e. is there difference between 
> > `-llvm-*` and `-ll*` ?
>
> it would be if they only do it to -llm-* (which could possibly catch most 
> cases)
>
> but what if someone is doing
>
> -llvm-header-guard
>
> We wouldn't catch those..
>
> This is a snippet of a .clang-tidy file from a  project I work on, we turn 
> off specific checks by fully qualifying the checker
>
>   
>   llvm-*,
>   -llvm-header-guard,
>   -llvm-include-order,
>   misc-*,
>   modernize-*,
>   ...
>


You make a great point.  I'll look into just changing the namespace name.  I 
wasn't really comfortable with all the code churn it involved anyway, but once 
I got started, I figured I'd finish it.  The duplicate `llvm` namespace is the 
issue, not other names.  Thanks for the feedback...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60151



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

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

@klimek, this patch is now ready for review.  Thanks...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



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

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 194729.
hintonda added a comment.

Enhance Option::reset to reset the default value and remove the option
if it's a cl::DefaultOption.

Also, make sure removeOption only removes an option if both the name
and the Option matches, not just the name -- different Options with
the same name in different SubCommands have always been possible, but
are more common with DefaultOptions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746

Files:
  clang/lib/Tooling/CommonOptionsParser.cpp
  llvm/docs/CommandLine.rst
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp
  llvm/test/Support/check-default-options.txt
  llvm/tools/llvm-opt-report/OptReport.cpp
  llvm/unittests/Support/CommandLineTest.cpp

Index: llvm/unittests/Support/CommandLineTest.cpp
===
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -620,6 +620,67 @@
   }
 }
 
+TEST(CommandLineTest, DefaultOptions) {
+  cl::ResetCommandLineParser();
+
+  StackOption Bar("bar", cl::sub(*cl::AllSubCommands),
+   cl::DefaultOption);
+  StackOption Bar_Alias(
+  "b", cl::desc("Alias for -bar"), cl::aliasopt(Bar), cl::DefaultOption);
+
+  StackOption Foo("foo", cl::init(false), cl::sub(*cl::AllSubCommands),
+cl::DefaultOption);
+  StackOption Foo_Alias("f", cl::desc("Alias for -foo"),
+ cl::aliasopt(Foo), cl::DefaultOption);
+
+  StackSubCommand SC1("sc1", "First Subcommand");
+  // Override "-b" and change type in sc1 SubCommand.
+  StackOption SC1_B("b", cl::sub(SC1), cl::init(false));
+  StackSubCommand SC2("sc2", "Second subcommand");
+  // Override "-foo" and change type in sc2 SubCommand.  Note that this does not
+  // affect "-f" alias, which continues to work correctly.
+  StackOption SC2_Foo("foo", cl::sub(SC2));
+
+  const char *args0[] = {"prog", "-b", "args0 bar string", "-f"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args0) / sizeof(char *), args0,
+  StringRef(), &llvm::nulls()));
+  EXPECT_TRUE(Bar == "args0 bar string");
+  EXPECT_TRUE(Foo);
+  EXPECT_FALSE(SC1_B);
+  EXPECT_TRUE(SC2_Foo.empty());
+
+  cl::ResetAllOptionOccurrences();
+
+  const char *args1[] = {"prog", "sc1", "-b", "-bar", "args1 bar string", "-f"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args1) / sizeof(char *), args1,
+  StringRef(), &llvm::nulls()));
+  EXPECT_TRUE(Bar == "args1 bar string");
+  EXPECT_TRUE(Foo);
+  EXPECT_TRUE(SC1_B);
+  EXPECT_TRUE(SC2_Foo.empty());
+  for (auto *S : cl::getRegisteredSubcommands()) {
+if (*S) {
+  EXPECT_EQ("sc1", S->getName());
+}
+  }
+
+  cl::ResetAllOptionOccurrences();
+
+  const char *args2[] = {"prog", "sc2", "-b", "args2 bar string",
+ "-f", "-foo", "foo string"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args2) / sizeof(char *), args2,
+  StringRef(), &llvm::nulls()));
+  EXPECT_TRUE(Bar == "args2 bar string");
+  EXPECT_TRUE(Foo);
+  EXPECT_FALSE(SC1_B);
+  EXPECT_TRUE(SC2_Foo == "foo string");
+  for (auto *S : cl::getRegisteredSubcommands()) {
+if (*S) {
+  EXPECT_EQ("sc2", S->getName());
+}
+  }
+}
+
 TEST(CommandLineTest, ArgumentLimit) {
   std::string args(32 * 4096, 'a');
   EXPECT_FALSE(llvm::sys::commandLineFitsWithinSystemLimits("cl", args.data()));
Index: llvm/tools/llvm-opt-report/OptReport.cpp
===
--- llvm/tools/llvm-opt-report/OptReport.cpp
+++ llvm/tools/llvm-opt-report/OptReport.cpp
@@ -36,8 +36,6 @@
 using namespace llvm;
 using namespace llvm::yaml;
 
-static cl::opt Help("h", cl::desc("Alias for -help"), cl::Hidden);
-
 // Mark all our options with this category, everything else (except for -version
 // and -help) will be hidden.
 static cl::OptionCategory
@@ -440,11 +438,6 @@
   "A tool to generate an optimization report from YAML optimization"
   " record files.\n");
 
-  if (Help) {
-cl::PrintHelpMessage();
-return 0;
-  }
-
   LocationInfoTy LocationInfo;
   if (!readLocationInfo(LocationInfo))
 return 1;
Index: llvm/test/Support/check-default-options.txt
===
--- /dev/null
+++ llvm/test/Support/check-default-options.txt
@@ -0,0 +1,18 @@
+# RUN: llvm-objdump -help-hidden %t | FileCheck --check-prefix=CHECK-OBJDUMP %s
+# RUN: llvm-readobj -help-hidden %t | FileCheck --check-prefix=CHECK-READOBJ %s
+# RUN: llvm-tblgen -help-hidden %t | FileCheck --check-prefix=CHECK-TBLGEN %s
+# RUN: llvm-opt-report -help-hidden %t | FileCheck --check-prefix=CHECK-OPT-RPT %s
+# RUN: llvm-dwarfdump -help-hidden %t | FileCheck --check-prefix=CHECK-D