[PATCH] D61663: [clang-format] Fix a JavaScript import order bug.

[Daniel Jasper via Phabricator via cfe-commits]

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

Generally, upload patches with the full file as context (that will prevent 
Phabricator's "Context not available")
But this change looks good. Thank you.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61663



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

[PATCH] D61842: [clangd] [WIP] [Not ready for review] Semantic highlighting

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 199202.
nridge added a comment.

Add [clangd] to title


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61842

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/ClangdTests.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
  clang-tools-extra/clangd/unittests/TestFS.h
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -524,8 +524,10 @@
   MockCompilationDatabase CDB(BuildDir, RelPathPrefix);
 
   IgnoreDiagnostics DiagConsumer;
+  NoopSemanticHighlightingConsumer SHConsumer;
   MockFSProvider FS;
-  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+  ClangdServer Server(CDB, FS, DiagConsumer, SHConsumer,
+  ClangdServer::optsForTest());
 
   // Fill the filesystem.
   auto FooCpp = testPath("src/foo.cpp");
@@ -1194,8 +1196,10 @@
 TEST(GoToInclude, All) {
   MockFSProvider FS;
   IgnoreDiagnostics DiagConsumer;
+  NoopSemanticHighlightingConsumer SHConsumer;
   MockCompilationDatabase CDB;
-  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+  ClangdServer Server(CDB, FS, DiagConsumer, SHConsumer,
+  ClangdServer::optsForTest());
 
   auto FooCpp = testPath("foo.cpp");
   const char *SourceContents = R"cpp(
@@ -1269,8 +1273,10 @@
   // good preamble.
   MockFSProvider FS;
   IgnoreDiagnostics DiagConsumer;
+  NoopSemanticHighlightingConsumer SHConsumer;
   MockCompilationDatabase CDB;
-  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+  ClangdServer Server(CDB, FS, DiagConsumer, SHConsumer,
+  ClangdServer::optsForTest());
 
   auto FooCpp = testPath("foo.cpp");
   // The trigger locations must be the same.
Index: clang-tools-extra/clangd/unittests/TestFS.h
===
--- clang-tools-extra/clangd/unittests/TestFS.h
+++ clang-tools-extra/clangd/unittests/TestFS.h
@@ -57,6 +57,12 @@
   StringRef RelPathPrefix;
 };
 
+class NoopSemanticHighlightingConsumer : public SemanticHighlightingConsumer {
+public:
+  void onSemanticHighlightingReady(
+  PathRef, std::vector) override {}
+};
+
 // Returns an absolute (fake) test directory for this OS.
 const char *testRoot();
 
Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -684,7 +684,9 @@
   } CaptureTUStatus;
   MockFSProvider FS;
   MockCompilationDatabase CDB;
-  ClangdServer Server(CDB, FS, CaptureTUStatus, ClangdServer::optsForTest());
+  NoopSemanticHighlightingConsumer SHConsumer;
+  ClangdServer Server(CDB, FS, CaptureTUStatus, SHConsumer,
+  ClangdServer::optsForTest());
   Annotations Code("int m^ain () {}");
 
   // We schedule the following tasks in the queue:
Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -0,0 +1,407 @@
+//===-- SemanticHighlightingTests.cpp--*- C++ -*---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+#include "Annotations.h"
+#include "SemanticHighlighting.h"
+#include "TestTU.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+#include 
+
+namespace clang {
+namespace clangd {
+
+namespace {
+
+std::string kindAsString(SemanticHighlightingKind Kind) {
+  switch (Kind) {
+  case SemanticHighlightingKind::Class:
+return "Class";
+  case SemanticHighlightingKind::Enum:
+return "Enum";
+  case 

[PATCH] D61842: [WIP] [Not ready for review] Semantic highlighting

[Nathan Ridge via Phabricator via cfe-commits]

nridge created this revision.
Herald added subscribers: cfe-commits, kadircet, arphaman, mgrang, jkorous, 
javed.absar, mgorny.
Herald added a project: clang.
nridge planned changes to this revision.

Done:

- Basic plumbing
- Encoding of semantic highlighting tokens into the protocol format
- Computation of (most) semantic highlightings given an AST
- Unit test suite (for implemented highlightings)

Remains to be done:

- Highlightings for macro definitions and macro invocations
- Some special highlightings like overloaded operators
- End-to-end test to exercise the protocol bits


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61842

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/ClangdTests.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
  clang-tools-extra/clangd/unittests/TestFS.h
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -524,8 +524,10 @@
   MockCompilationDatabase CDB(BuildDir, RelPathPrefix);
 
   IgnoreDiagnostics DiagConsumer;
+  NoopSemanticHighlightingConsumer SHConsumer;
   MockFSProvider FS;
-  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+  ClangdServer Server(CDB, FS, DiagConsumer, SHConsumer,
+  ClangdServer::optsForTest());
 
   // Fill the filesystem.
   auto FooCpp = testPath("src/foo.cpp");
@@ -1194,8 +1196,10 @@
 TEST(GoToInclude, All) {
   MockFSProvider FS;
   IgnoreDiagnostics DiagConsumer;
+  NoopSemanticHighlightingConsumer SHConsumer;
   MockCompilationDatabase CDB;
-  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+  ClangdServer Server(CDB, FS, DiagConsumer, SHConsumer,
+  ClangdServer::optsForTest());
 
   auto FooCpp = testPath("foo.cpp");
   const char *SourceContents = R"cpp(
@@ -1269,8 +1273,10 @@
   // good preamble.
   MockFSProvider FS;
   IgnoreDiagnostics DiagConsumer;
+  NoopSemanticHighlightingConsumer SHConsumer;
   MockCompilationDatabase CDB;
-  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+  ClangdServer Server(CDB, FS, DiagConsumer, SHConsumer,
+  ClangdServer::optsForTest());
 
   auto FooCpp = testPath("foo.cpp");
   // The trigger locations must be the same.
Index: clang-tools-extra/clangd/unittests/TestFS.h
===
--- clang-tools-extra/clangd/unittests/TestFS.h
+++ clang-tools-extra/clangd/unittests/TestFS.h
@@ -57,6 +57,12 @@
   StringRef RelPathPrefix;
 };
 
+class NoopSemanticHighlightingConsumer : public SemanticHighlightingConsumer {
+public:
+  void onSemanticHighlightingReady(
+  PathRef, std::vector) override {}
+};
+
 // Returns an absolute (fake) test directory for this OS.
 const char *testRoot();
 
Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -684,7 +684,9 @@
   } CaptureTUStatus;
   MockFSProvider FS;
   MockCompilationDatabase CDB;
-  ClangdServer Server(CDB, FS, CaptureTUStatus, ClangdServer::optsForTest());
+  NoopSemanticHighlightingConsumer SHConsumer;
+  ClangdServer Server(CDB, FS, CaptureTUStatus, SHConsumer,
+  ClangdServer::optsForTest());
   Annotations Code("int m^ain () {}");
 
   // We schedule the following tasks in the queue:
Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -0,0 +1,407 @@
+//===-- SemanticHighlightingTests.cpp--*- C++ -*---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//

[PATCH] D60953: [clangd] Respect clang-tidy suppression comments

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

Friendly review ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60953



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

[PATCH] D61841: [clangd] Respect WarningsAsErrors configuration for clang-tidy

[Nathan Ridge via Phabricator via cfe-commits]

nridge created this revision.
nridge added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.

This completes the fix for https://bugs.llvm.org/show_bug.cgi?id=41218.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61841

Files:
  clang-tools-extra/clangd/ClangdUnit.cpp

Index: clang-tools-extra/clangd/ClangdUnit.cpp
===
--- clang-tools-extra/clangd/ClangdUnit.cpp
+++ clang-tools-extra/clangd/ClangdUnit.cpp
@@ -113,12 +113,12 @@
   }
 
   void EndOfMainFile() {
-for (const auto& Entry : MainFileMacros)
+for (const auto  : MainFileMacros)
   Out->push_back(Entry.getKey());
 llvm::sort(*Out);
   }
 
- private:
+private:
   const SourceManager 
   bool InMainFile = true;
   llvm::StringSet<> MainFileMacros;
@@ -275,12 +275,37 @@
   const LangOptions 
 };
 
+// This is a copy of ClangTidyContext::CachedGlobList.
+// FIXME: Factor out to a common place.
+class CachedGlobList {
+public:
+  CachedGlobList(StringRef Globs) : Globs(Globs) {}
+
+  bool contains(StringRef S) {
+switch (auto  = Cache[S]) {
+case Yes:
+  return true;
+case No:
+  return false;
+case None:
+  Result = Globs.contains(S) ? Yes : No;
+  return Result == Yes;
+}
+llvm_unreachable("invalid enum");
+  }
+
+private:
+  tidy::GlobList Globs;
+  enum Tristate { None, Yes, No };
+  llvm::StringMap Cache;
+};
+
 // A wrapper around StoreDiags to handle suppression comments for
 // clang-tidy diagnostics (and possibly other clang-tidy customizations in the
 // future).
 class ClangdDiagnosticConsumer : public StoreDiags {
 public:
-  ClangdDiagnosticConsumer() {
+  ClangdDiagnosticConsumer(llvm::Optional WarningsAsErrors) {
 filterDiagnostics([this](DiagnosticsEngine::Level DiagLevel,
  const clang::Diagnostic ,
  bool IsInsideMainFile) {
@@ -299,14 +324,36 @@
   }
   return true;
 });
+
+if (WarningsAsErrors) {
+  WarningAsErrorFilter =
+  llvm::make_unique(*WarningsAsErrors);
+}
   }
 
   void setClangTidyContext(tidy::ClangTidyContext *CTContext) {
 this->CTContext = CTContext;
   }
 
+  void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
+const clang::Diagnostic ) override {
+DiagnosticsEngine::Level Level = DiagLevel;
+
+// Check for warning-as-error.
+if (CTContext && WarningAsErrorFilter &&
+DiagLevel == DiagnosticsEngine::Warning) {
+  std::string CheckName = CTContext->getCheckName(Info.getID());
+  if (!CheckName.empty() && WarningAsErrorFilter->contains(CheckName)) {
+Level = DiagnosticsEngine::Error;
+  }
+}
+
+StoreDiags::HandleDiagnostic(Level, Info);
+  }
+
 private:
   tidy::ClangTidyContext *CTContext = nullptr;
+  std::unique_ptr WarningAsErrorFilter;
 };
 
 } // namespace
@@ -328,7 +375,7 @@
   const PrecompiledPreamble *PreamblePCH =
   Preamble ? >Preamble : nullptr;
 
-  ClangdDiagnosticConsumer ASTDiags;
+  ClangdDiagnosticConsumer ASTDiags{Opts.ClangTidyOpts.WarningsAsErrors};
   std::string Content = Buffer->getBuffer();
 
   auto Clang = prepareCompilerInstance(std::move(CI), PreamblePCH,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61839: [analyzer][WIP] Hide developer-only checker/package options by default

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus created this revision.
Szelethus added reviewers: NoQ, dcoughlin, xazax.hun, rnkovacs, Charusso, 
a_sidorin, baloghadamsoftware.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy, 
mikhail.ramalho, a.sidorin, szepet, whisperity.
Szelethus added a parent revision: D57858: [analyzer] Add a new frontend flag 
to display all checker options.
Szelethus edited the summary of this revision.

Followup to D60925 , this time, for checker 
options. Non-checker configurations should receive the same treatment 
eventually.

The patch is marked WIP, because I am not really confident about osx checkers, 
and I can see valid uses for the rest of the options, but I might not be strict 
enough on this. Also, @NoQ suggested that hidden checker's options should be 
hidden by default, but I'm kind of hesitant about it, as described in 
D57858#1499414 .


Repository:
  rC Clang

https://reviews.llvm.org/D61839

Files:
  include/clang/Driver/CC1Options.td
  include/clang/StaticAnalyzer/Checkers/CheckerBase.td
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
  lib/Frontend/CompilerInvocation.cpp
  lib/FrontendTool/ExecuteCompilerInvocation.cpp
  lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
  test/Analysis/analyzer-checker-option-help.c
  utils/TableGen/ClangSACheckersEmitter.cpp

Index: utils/TableGen/ClangSACheckersEmitter.cpp
===
--- utils/TableGen/ClangSACheckersEmitter.cpp
+++ utils/TableGen/ClangSACheckersEmitter.cpp
@@ -113,6 +113,7 @@
 static bool isHidden(const Record *R) {
   if (R->getValueAsBit("Hidden"))
 return true;
+
   // Not declared as hidden, check the parent package if it is hidden.
   if (DefInit *DI = dyn_cast(R->getValueInit("ParentPackage")))
 return isHidden(DI->getDef());
@@ -121,21 +122,38 @@
 }
 
 static void printChecker(llvm::raw_ostream , const Record ) {
-OS << "CHECKER(" << "\"";
-OS.write_escaped(getCheckerFullName()) << "\", ";
-OS << R.getName() << ", ";
-OS << "\"";
-OS.write_escaped(getStringValue(R, "HelpText")) << "\", ";
-OS << "\"";
-OS.write_escaped(getCheckerDocs(R));
-OS << "\", ";
-
-if (!isHidden())
-  OS << "false";
-else
-  OS << "true";
+  OS << "CHECKER(" << "\"";
+  OS.write_escaped(getCheckerFullName()) << "\", ";
+  OS << R.getName() << ", ";
+  OS << "\"";
+  OS.write_escaped(getStringValue(R, "HelpText")) << "\", ";
+  OS << "\"";
+  OS.write_escaped(getCheckerDocs(R));
+  OS << "\", ";
+
+  if (!isHidden())
+OS << "false";
+  else
+OS << "true";
+
+  OS << ")\n";
+}
+
+static void printOption(llvm::raw_ostream , StringRef FullName,
+const Record ) {
+  OS << "\"";
+  OS.write_escaped(getCheckerOptionType(R)) << "\", \"";
+  OS.write_escaped(FullName) << "\", ";
+  OS << '\"' << getStringValue(R, "CmdFlag") << "\", ";
+  OS << '\"';
+  OS.write_escaped(getStringValue(R, "Desc")) << "\", ";
+  OS << '\"';
+  OS.write_escaped(getStringValue(R, "DefaultVal")) << "\", ";
 
-OS << ")\n";
+  if (!R.getValueAsBit("Hidden"))
+OS << "false";
+  else
+OS << "true";
 }
 
 namespace clang {
@@ -196,14 +214,8 @@
 std::vector PackageOptions = Package
->getValueAsListOfDefs("PackageOptions");
 for (Record *PackageOpt : PackageOptions) {
-  OS << "PACKAGE_OPTION(\"";
-  OS.write_escaped(getCheckerOptionType(*PackageOpt)) << "\", \"";
-  OS.write_escaped(getPackageFullName(Package)) << "\", ";
-  OS << '\"' << getStringValue(*PackageOpt, "CmdFlag") << "\", ";
-  OS << '\"';
-  OS.write_escaped(getStringValue(*PackageOpt, "Desc")) << "\", ";
-  OS << '\"';
-  OS.write_escaped(getStringValue(*PackageOpt, "DefaultVal")) << "\"";
+  OS << "PACKAGE_OPTION(";
+  printOption(OS, getPackageFullName(Package), *PackageOpt);
   OS << ")\n";
 }
   }
@@ -277,16 +289,9 @@
 std::vector CheckerOptions = Checker
->getValueAsListOfDefs("CheckerOptions");
 for (Record *CheckerOpt : CheckerOptions) {
-  OS << "CHECKER_OPTION(\"";
-  OS << getCheckerOptionType(*CheckerOpt) << "\", \"";
-  OS.write_escaped(getCheckerFullName(Checker)) << "\", ";
-  OS << '\"' << getStringValue(*CheckerOpt, "CmdFlag") << "\", ";
-  OS << '\"';
-  OS.write_escaped(getStringValue(*CheckerOpt, "Desc")) << "\", ";
-  OS << '\"';
-  OS.write_escaped(getStringValue(*CheckerOpt, "DefaultVal")) << "\"";
-  OS << ")";
-  OS << '\n';
+  OS << "CHECKER_OPTION(";
+  printOption(OS, getCheckerFullName(Checker), *CheckerOpt);
+  OS << ")\n";
 }
   }
   OS << "#endif // GET_CHECKER_OPTIONS\n"
Index: 

[PATCH] D57858: [analyzer] Add a new frontend flag to display all checker options

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a subscriber: o.gyorgy.
Szelethus added a comment.

In D57858#1498640 , @NoQ wrote:

> So, like, the global picture is as follows. In our case the Driver (i.e., 
> --analyze) is not much more user facing than frontend flags. It's still 
> fairly unusable directly, as our Static Analyzer is generally not a 
> command-line tool. The real user-facing stuff is the GUIs such as scan-build 
> or CodeChecker. These GUIs decide themselves on what options they want to 
> expose. For instance, you have a right to decide that CodeChecker shouldn't 
> support the aggressive mode of the move-checker and don't expose it as an 
> option in your GUI. In this sense it's not really useful to provide a 
> centralized `-help` of all user-facing options.
>
> But it sounds as if you want to change this situation and provide a single 
> source of truth on what are the user-facing options. Particular GUIs can 
> still ignore them, but you don't want to hardcode flags in CodeChecker, but 
> instead you want to rely on clang to provide the list of supported options 
> for you and, as a side effect, for scan-build users (if you also add a 
> scan-build help flag). I'm totally in favor of crystallizing such list of 
> user-facing flags, and this patch sounds like a good step in that direction, 
> assuming that non-user-facing options are hidden.


That describes my intention quite well :)

> I'm still in favor of hiding alpha checkers (as they are for development 
> only, much like debug flags; i'd recommend hiding them in the CodeChecker UI 
> as well)
> 
> Now, why do we care about frontend/driver flags when they're unusable by 
> definition? That's because we have a mental trauma after seeing a few 
> powerusers actively explore those flags, observe that they don't work, and 
> then tell everybody that the Analyzer is broken. So there's a threshold, 
> based on a tiny but painful bit of practical experience, that says that 
> documentation of developer-only features on llvm.org or in code comments is 
> fine, but documentation printed by the released binary itself is not fine.

What you say sounds very reasonable. Still, I am kind of hesitant about hiding 
all alpha checkers: I initially intended to hide checkers that are 
developer-only checkers (modeling, debug). I guess if we define alpha checkers 
(as you stated numerous times) as incomplete, under development checkers, that 
are missing half their limbs and crash if you look at them the wrong way, sure, 
they belong in the developer-only category. But checkers such as mine 
(UninitializedObjectChecker), for the longest time were very stable, have been 
enabled by default for our internal projects, despite only recently moving out 
of alpha.

Then agaaain, if we're that stubborn about alpha checkers, we could might as 
well dig them out of `-analyzer-checker-help`, and leave the rest there. 
Untangling what alpha checkers depend on one another could be solved by making 
yet another frontend flag that would display checker dependencies, which would 
be super easy since D54438 . @dkrupp @o.gyorgy 
Do you have any feelings on this?

> and we should probably automatically hide options of checker that are hidden.

Checker options are a different kind of animal entirely. think we should 
definitely let the user fine-tune some modeling checkers, for instance, 
`unix.DynamicMemoryModeling:Optimistic`, despite us not really wanting to let  
anyone (even developers really) mess around whether 
`unix.DynamicMemoryModeling` should be enabled. While that specific option is, 
to put it nicely, a little esoteric, making some decisions the analyzer makes 
less conservative, or limiting state splits to help performance may be 
desirable in the future.

Let's move the rest of the discussion directly related to hiding checker 
options to D61839 !


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

https://reviews.llvm.org/D57858



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

[PATCH] D61838: [Sema] Suppress additional warnings for C's zero initializer

[Alex James via Phabricator via cfe-commits]

al3xtjames updated this revision to Diff 199192.
al3xtjames edited the summary of this revision.
al3xtjames added a comment.

Formatting changes.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61838

Files:
  clang/lib/AST/Expr.cpp
  clang/test/Sema/zero-initializer.c


Index: clang/test/Sema/zero-initializer.c
===
--- clang/test/Sema/zero-initializer.c
+++ clang/test/Sema/zero-initializer.c
@@ -7,6 +7,8 @@
 struct B { struct A a; };
 struct C { struct B b; };
 struct D { struct C c; int n; };
+struct E { short e; };
+struct F { struct E e; int n; };
 
 int main(void)
 {
@@ -23,6 +25,9 @@
   struct C p = { 0 }; // no-warning
   struct C q = { 9 }; // warning suppressed for struct with single element
   struct D r = { 9 }; // expected-warning {{suggest braces around 
initialization of subobject}} expected-warning {{missing field 'n' initializer}}
+  struct F s = { 0 }; // no-warning
+  struct F t = { 9 }; // expected-warning {{suggest braces around 
initialization of subobject}} expected-warning {{missing field 'n' initializer}}
+
   f = (struct foo ) { 0 }; // no-warning
   g = (struct foo ) { 9 }; // expected-warning {{missing field 'y' 
initializer}}
   h = (struct foo ) { 9, 9 }; // no-warning
@@ -36,6 +41,8 @@
   p = (struct C) { 0 }; // no-warning
   q = (struct C) { 9 }; // warning suppressed for struct with single element
   r = (struct D) { 9 }; // expected-warning {{suggest braces around 
initialization of subobject}} expected-warning {{missing field 'n' initializer}}
+  s = (struct F) { 0 }; // no-warning
+  t = (struct F) { 9 }; // expected-warning {{suggest braces around 
initialization of subobject}} expected-warning {{missing field 'n' initializer}}
 
   return 0;
 }
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -2090,6 +2090,10 @@
   }
 
   const IntegerLiteral *Lit = dyn_cast(getInit(0));
+  if (!Lit) {
+if (const ImplicitCastExpr *ICE = dyn_cast(getInit(0)))
+  Lit = dyn_cast(ICE->getSubExpr());
+  }
   return Lit && Lit->getValue() == 0;
 }
 


Index: clang/test/Sema/zero-initializer.c
===
--- clang/test/Sema/zero-initializer.c
+++ clang/test/Sema/zero-initializer.c
@@ -7,6 +7,8 @@
 struct B { struct A a; };
 struct C { struct B b; };
 struct D { struct C c; int n; };
+struct E { short e; };
+struct F { struct E e; int n; };
 
 int main(void)
 {
@@ -23,6 +25,9 @@
   struct C p = { 0 }; // no-warning
   struct C q = { 9 }; // warning suppressed for struct with single element
   struct D r = { 9 }; // expected-warning {{suggest braces around initialization of subobject}} expected-warning {{missing field 'n' initializer}}
+  struct F s = { 0 }; // no-warning
+  struct F t = { 9 }; // expected-warning {{suggest braces around initialization of subobject}} expected-warning {{missing field 'n' initializer}}
+
   f = (struct foo ) { 0 }; // no-warning
   g = (struct foo ) { 9 }; // expected-warning {{missing field 'y' initializer}}
   h = (struct foo ) { 9, 9 }; // no-warning
@@ -36,6 +41,8 @@
   p = (struct C) { 0 }; // no-warning
   q = (struct C) { 9 }; // warning suppressed for struct with single element
   r = (struct D) { 9 }; // expected-warning {{suggest braces around initialization of subobject}} expected-warning {{missing field 'n' initializer}}
+  s = (struct F) { 0 }; // no-warning
+  t = (struct F) { 9 }; // expected-warning {{suggest braces around initialization of subobject}} expected-warning {{missing field 'n' initializer}}
 
   return 0;
 }
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -2090,6 +2090,10 @@
   }
 
   const IntegerLiteral *Lit = dyn_cast(getInit(0));
+  if (!Lit) {
+if (const ImplicitCastExpr *ICE = dyn_cast(getInit(0)))
+  Lit = dyn_cast(ICE->getSubExpr());
+  }
   return Lit && Lit->getValue() == 0;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57858: [analyzer] Add a new frontend flag to display all checker options

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

In D57858#1498651 , @NoQ wrote:

> Also, for CodeChecker purposes wouldn't you rather have a machine-readable 
> dump?


My main purpose was readability in the terminal, but I will need to think about 
that eventually. The current diff allows such formatting changes very easily, 
should CodeChecker have any issues, and I do like the output format as it is 
currently (as far as I know, clang-tidy check options are formatted similarly).


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

https://reviews.llvm.org/D57858



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

[PATCH] D57858: [analyzer] Add a new frontend flag to display all checker options

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus updated this revision to Diff 199188.
Szelethus added a comment.

- Make the educated observation that the code for formatted output is 
duplicated for `-analyzer-checker-help`, `-analyzer-config-help` and this 
patch, reuse it via the new static function 
`AnalyzerOptions::printFormattedEntry`.
- Remove the `USAGE` that suggested the use of the driver (did you mean to 
remove it entirely? Really, without reading test files, you'd never know how to 
use it)
- Hide certain options by default, implemented in a followup patch.


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

https://reviews.llvm.org/D57858

Files:
  include/clang/Driver/CC1Options.td
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
  include/clang/StaticAnalyzer/Frontend/FrontendActions.h
  lib/Frontend/CompilerInvocation.cpp
  lib/FrontendTool/ExecuteCompilerInvocation.cpp
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
  lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
  test/Analysis/analyzer-checker-option-help.c
  test/Analysis/analyzer-list-configs.c
  test/Analysis/checker-plugins.c

Index: test/Analysis/checker-plugins.c
===
--- test/Analysis/checker-plugins.c
+++ test/Analysis/checker-plugins.c
@@ -104,3 +104,12 @@
 // RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-CORRECTED-BOOL-VALUE
 
 // CHECK-CORRECTED-BOOL-VALUE: example.MyChecker:ExampleOption = false
+
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -load %llvmshlibdir/CheckerOptionHandlingAnalyzerPlugin%pluginext\
+// RUN:   -analyzer-checker=example.MyChecker \
+// RUN:   -analyzer-checker-option-help \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-CHECKER-OPTION-HELP
+
+// CHECK-CHECKER-OPTION-HELP: example.MyChecker:ExampleOption  (bool) This is an
+// CHECK-CHECKER-OPTION-HELP-SAME: example checker opt. (default: false)
Index: test/Analysis/analyzer-list-configs.c
===
--- test/Analysis/analyzer-list-configs.c
+++ test/Analysis/analyzer-list-configs.c
@@ -1,14 +1,11 @@
 // RUN: %clang_cc1 -analyzer-config-help 2>&1 | FileCheck %s
+
 // CHECK: OVERVIEW: Clang Static Analyzer -analyzer-config Option List
 //
-// CHECK: USAGE: clang -cc1 [CLANG_OPTIONS] -analyzer-config 
-//
-// CHECK:  clang -cc1 [CLANG_OPTIONS] -analyzer-config OPTION1=VALUE, -analyzer-config OPTION2=VALUE, ...
-//
-// CHECK:  clang [CLANG_OPTIONS] -Xclang -analyzer-config -Xclang
-//
-// CHECK:  clang [CLANG_OPTIONS] -Xclang -analyzer-config -Xclang OPTION1=VALUE, -Xclang -analyzer-config -Xclang OPTION2=VALUE, ...
+// CHECK: USAGE: -analyzer-config 
 //
+// CHECK:-analyzer-config OPTION1=VALUE, -analyzer-config
+// CHECK-SAME:   OPTION2=VALUE, ...
 //
 // CHECK: OPTIONS:
 //
Index: test/Analysis/analyzer-checker-option-help.c
===
--- /dev/null
+++ test/Analysis/analyzer-checker-option-help.c
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -analyzer-checker-option-help 2>&1 | FileCheck %s
+
+// CHECK: OVERVIEW: Clang Static Analyzer Checker and Package Option List
+//
+// CHECK: USAGE: -analyzer-config 
+//
+// CHECK:-analyzer-config OPTION1=VALUE, -analyzer-config
+// CHECK-SAME:   OPTION2=VALUE, ...
+//
+// CHECK: OPTIONS:
+//
+// CHECK:   alpha.clone.CloneChecker:MinimumCloneComplexity
+// CHECK-SAME:   (int) Ensures that every clone has at least
+// CHECK:the given complexity. Complexity is here
+// CHECK:defined as the total amount of children
+// CHECK:of a statement. This constraint assumes
+// CHECK:the first statement in the group is representative
+// CHECK:for all other statements in the group in
+// CHECK:terms of complexity. (default: 50)
Index: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
===
--- lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
+++ lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
@@ -518,17 +518,8 @@
 if (!AnOpts.ShowCheckerHelpHidden && Checker.IsHidden)
   continue;
 
-Out.indent(InitialPad) << Checker.FullName;
-
-int Pad = OptionFieldWidth - Checker.FullName.size();
-
-// Break on long option names.
-if (Pad < 0) {
-  Out << '\n';
-  Pad = OptionFieldWidth + InitialPad;
-}
-Out.indent(Pad + 2) << Checker.Desc;
-
+AnalyzerOptions::printFormattedEntry(Out, {Checker.FullName, Checker.Desc},
+ InitialPad, OptionFieldWidth);
 Out << '\n';
   }
 }
@@ -540,3 +531,41 @@
   for (const auto *i : EnabledCheckers)
 Out << i->FullName << '\n';
 }
+
+void CheckerRegistry::printCheckerOptionList(raw_ostream ) const {
+  Out << "OVERVIEW: Clang Static Analyzer Checker and Package Option List\n\n";
+  Out << "USAGE: -analyzer-config \n\n";

r360548 - Fix test to use -cc1.

[Leonard Chan via cfe-commits]

Author: leonardchan
Date: Sun May 12 15:44:46 2019
New Revision: 360548

URL: http://llvm.org/viewvc/llvm-project?rev=360548=rev
Log:
Fix test to use -cc1.

Added:
cfe/trunk/test/Sema/mingw-macro-qualified-type.c
Removed:
cfe/trunk/test/Driver/mingw-macro-qualified-type.c

Removed: cfe/trunk/test/Driver/mingw-macro-qualified-type.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/mingw-macro-qualified-type.c?rev=360547=auto
==
--- cfe/trunk/test/Driver/mingw-macro-qualified-type.c (original)
+++ cfe/trunk/test/Driver/mingw-macro-qualified-type.c (removed)
@@ -1,12 +0,0 @@
-// Ensure that builtin attributes do not get treated as user defined macros to
-// be weapped in macro qualified types. This addresses P41852.
-//
-// RUN: %clang -c %s -target i686-w64-mingw32
-
-typedef int WINBOOL;
-typedef unsigned int UINT_PTR, *PUINT_PTR;
-typedef unsigned long long ULONG64, *PULONG64;
-#define WINAPI __stdcall
-#define CALLBACK __stdcall
-
-typedef WINBOOL(CALLBACK WINAPI *PSYMBOLSERVERCALLBACKPROC)(UINT_PTR action, 
ULONG64 data, ULONG64 context);

Added: cfe/trunk/test/Sema/mingw-macro-qualified-type.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/mingw-macro-qualified-type.c?rev=360548=auto
==
--- cfe/trunk/test/Sema/mingw-macro-qualified-type.c (added)
+++ cfe/trunk/test/Sema/mingw-macro-qualified-type.c Sun May 12 15:44:46 2019
@@ -0,0 +1,13 @@
+// Ensure that builtin attributes do not get treated as user defined macros to
+// be weapped in macro qualified types. This addresses P41852.
+//
+// RUN: %clang_cc1 %s -triple i686-w64-mingw32 -fsyntax-only -verify
+// expected-no-diagnostics
+
+typedef int WINBOOL;
+typedef unsigned int UINT_PTR, *PUINT_PTR;
+typedef unsigned long long ULONG64, *PULONG64;
+#define WINAPI __stdcall
+#define CALLBACK __stdcall
+
+typedef WINBOOL(CALLBACK WINAPI *PSYMBOLSERVERCALLBACKPROC)(UINT_PTR action, 
ULONG64 data, ULONG64 context);


___
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] D51329: [Attribute/Diagnostics] Print macro if definition is an attribute declaration

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

In D51329#1499307 , @mstorsjo wrote:

> This caused a regression when including mingw-w64 headers (in a case where 
> they happen to include redundant/duplicate attribute specifications), 
> triggering asserts. See https://bugs.llvm.org/show_bug.cgi?id=41852 for 
> details.


Thanks for the alert. I submitted r360544 which should address this.


Repository:
  rC Clang

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

https://reviews.llvm.org/D51329



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

[PATCH] D61769: [clang] Regenerate AST matcher docs 

[Stephane Moore via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL360545: [clang] Regenerate AST matcher docs  (authored by 
stephanemoore, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D61769?vs=198941=199187#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61769

Files:
  cfe/trunk/docs/LibASTMatchersReference.html


Index: cfe/trunk/docs/LibASTMatchersReference.html
===
--- cfe/trunk/docs/LibASTMatchersReference.html
+++ cfe/trunk/docs/LibASTMatchersReference.html
@@ -3616,30 +3616,6 @@
 
 
 
-Matcherhttps://clang.llvm.org/doxygen/classclang_1_1ObjCMethodDecl.html;>ObjCMethodDeclisClassMethod
-Returns true when the 
Objective-C method declaration is a class method.
-
-Example
-matcher = objcMethodDecl(isClassMethod())
-matches
-  @interface I + (void)foo; @end
-but not
-  @interface I - (void)bar; @end
-
-
-
-Matcherhttps://clang.llvm.org/doxygen/classclang_1_1ObjCMethodDecl.html;>ObjCMethodDeclisInstanceMethod
-Returns true when 
the Objective-C method declaration is an instance method.
-
-Example
-matcher = objcMethodDecl(isInstanceMethod())
-matches
-  @interface I - (void)bar; @end
-but not
-  @interface I + (void)foo; @end
-
-
-
 Matcherhttps://clang.llvm.org/doxygen/classclang_1_1ObjCMessageExpr.html;>ObjCMessageExprmatchesSelectorstd::string 
RegExp
 Matches ObjC 
selectors whose name contains
 a substring matched by the given RegExp.
@@ -3662,6 +3638,18 @@
 
 
 
+Matcherhttps://clang.llvm.org/doxygen/classclang_1_1ObjCMethodDecl.html;>ObjCMethodDeclisClassMethod
+Returns true when the 
Objective-C method declaration is a class method.
+
+Example
+matcher = objcMethodDecl(isClassMethod())
+matches
+@interface I + (void)foo; @end
+but not
+@interface I - (void)bar; @end
+
+
+
 Matcherhttps://clang.llvm.org/doxygen/classclang_1_1ObjCMethodDecl.html;>ObjCMethodDeclisDefinition
 Matches if a 
declaration has a body attached.
 
@@ -3684,6 +3672,18 @@
 
 
 
+Matcherhttps://clang.llvm.org/doxygen/classclang_1_1ObjCMethodDecl.html;>ObjCMethodDeclisInstanceMethod
+Returns true when 
the Objective-C method declaration is an instance method.
+
+Example
+matcher = objcMethodDecl(isInstanceMethod())
+matches
+@interface I - (void)bar; @end
+but not
+@interface I + (void)foo; @end
+
+
+
 Matcherhttps://clang.llvm.org/doxygen/classclang_1_1ParmVarDecl.html;>ParmVarDeclhasDefaultArgument
 Matches a 
declaration that has default arguments.
 


Index: cfe/trunk/docs/LibASTMatchersReference.html
===
--- cfe/trunk/docs/LibASTMatchersReference.html
+++ cfe/trunk/docs/LibASTMatchersReference.html
@@ -3616,30 +3616,6 @@
 
 
 
-Matcherhttps://clang.llvm.org/doxygen/classclang_1_1ObjCMethodDecl.html;>ObjCMethodDeclisClassMethod
-Returns true when the Objective-C method declaration is a class method.
-
-Example
-matcher = objcMethodDecl(isClassMethod())
-matches
-  @interface I + (void)foo; @end
-but not
-  @interface I - (void)bar; @end
-
-
-
-Matcherhttps://clang.llvm.org/doxygen/classclang_1_1ObjCMethodDecl.html;>ObjCMethodDeclisInstanceMethod
-Returns true when the Objective-C method declaration is an instance method.
-
-Example
-matcher = objcMethodDecl(isInstanceMethod())
-matches
-  @interface I - (void)bar; @end
-but not
-  @interface I + (void)foo; @end
-
-
-
 Matcherhttps://clang.llvm.org/doxygen/classclang_1_1ObjCMessageExpr.html;>ObjCMessageExprmatchesSelectorstd::string RegExp
 Matches ObjC selectors whose name contains
 a substring matched by the given RegExp.
@@ -3662,6 +3638,18 @@
 
 
 
+Matcherhttps://clang.llvm.org/doxygen/classclang_1_1ObjCMethodDecl.html;>ObjCMethodDeclisClassMethod
+Returns true when the Objective-C method declaration is a class method.
+
+Example
+matcher = objcMethodDecl(isClassMethod())
+matches
+@interface I + (void)foo; @end
+but not
+@interface I - (void)bar; @end
+
+
+
 Matcherhttps://clang.llvm.org/doxygen/classclang_1_1ObjCMethodDecl.html;>ObjCMethodDeclisDefinition
 Matches if a declaration has a body attached.
 
@@ -3684,6 +3672,18 @@
 
 
 
+Matcherhttps://clang.llvm.org/doxygen/classclang_1_1ObjCMethodDecl.html;>ObjCMethodDeclisInstanceMethod
+Returns true when the Objective-C method declaration is an instance method.
+
+Example
+matcher = objcMethodDecl(isInstanceMethod())
+matches
+@interface I - (void)bar; @end
+but not
+@interface I + (void)foo; @end
+
+
+
 Matcherhttps://clang.llvm.org/doxygen/classclang_1_1ParmVarDecl.html;>ParmVarDeclhasDefaultArgument
 Matches a declaration that has default arguments.
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r360544 - Fix for P41852 where builtin attributes were being caught by FindLocsWithCommonFileID().

[Leonard Chan via cfe-commits]

Author: leonardchan
Date: Sun May 12 14:50:01 2019
New Revision: 360544

URL: http://llvm.org/viewvc/llvm-project?rev=360544=rev
Log:
Fix for P41852 where builtin attributes were being caught by 
FindLocsWithCommonFileID().

Added:
cfe/trunk/test/Driver/mingw-macro-qualified-type.c
Modified:
cfe/trunk/lib/Parse/ParseDecl.cpp

Modified: cfe/trunk/lib/Parse/ParseDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=360544=360543=360544=diff
==
--- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDecl.cpp Sun May 12 14:50:01 2019
@@ -224,8 +224,9 @@ void Parser::ParseGNUAttributes(ParsedAt
 
 // If this was declared in a macro, attach the macro IdentifierInfo to the
 // parsed attribute.
-if (FindLocsWithCommonFileID(PP, AttrTokLoc, Loc)) {
-  auto  = PP.getSourceManager();
+auto  = PP.getSourceManager();
+if (!SM.isWrittenInBuiltinFile(SM.getSpellingLoc(AttrTokLoc)) &&
+FindLocsWithCommonFileID(PP, AttrTokLoc, Loc)) {
   CharSourceRange ExpansionRange = SM.getExpansionRange(AttrTokLoc);
   StringRef FoundName =
   Lexer::getSourceText(ExpansionRange, SM, PP.getLangOpts());

Added: cfe/trunk/test/Driver/mingw-macro-qualified-type.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/mingw-macro-qualified-type.c?rev=360544=auto
==
--- cfe/trunk/test/Driver/mingw-macro-qualified-type.c (added)
+++ cfe/trunk/test/Driver/mingw-macro-qualified-type.c Sun May 12 14:50:01 2019
@@ -0,0 +1,12 @@
+// Ensure that builtin attributes do not get treated as user defined macros to
+// be weapped in macro qualified types. This addresses P41852.
+//
+// RUN: %clang -c %s -target i686-w64-mingw32
+
+typedef int WINBOOL;
+typedef unsigned int UINT_PTR, *PUINT_PTR;
+typedef unsigned long long ULONG64, *PULONG64;
+#define WINAPI __stdcall
+#define CALLBACK __stdcall
+
+typedef WINBOOL(CALLBACK WINAPI *PSYMBOLSERVERCALLBACKPROC)(UINT_PTR action, 
ULONG64 data, ULONG64 context);


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

[PATCH] D61837: Make it possible control matcher traversal kind with ASTContext

[Stephen Kelly via Phabricator via cfe-commits]

steveire updated this revision to Diff 199185.
steveire added a comment.

Format


Repository:
  rC Clang

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

https://reviews.llvm.org/D61837

Files:
  include/clang/AST/ASTContext.h
  include/clang/AST/ASTNodeTraverser.h
  include/clang/ASTMatchers/ASTMatchers.h
  include/clang/ASTMatchers/ASTMatchersInternal.h
  lib/AST/ASTContext.cpp
  lib/ASTMatchers/ASTMatchFinder.cpp
  lib/ASTMatchers/ASTMatchersInternal.cpp
  unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -1510,6 +1510,72 @@
   notMatches("class C {}; C a = C();", varDecl(has(cxxConstructExpr();
 }
 
+TEST(Traversal, traverseMatcher) {
+
+  StringRef VarDeclCode = R"cpp(
+void foo()
+{
+  int i = 3.0;
+}
+)cpp";
+
+  auto Matcher = varDecl(hasInitializer(floatLiteral()));
+
+  EXPECT_TRUE(
+  notMatches(VarDeclCode, traverse(ast_type_traits::TK_AsIs, Matcher)));
+  EXPECT_TRUE(
+  matches(VarDeclCode,
+  traverse(ast_type_traits::TK_IgnoreImplicitCastsAndParentheses,
+   Matcher)));
+}
+
+TEST(Traversal, traverseMatcherNesting) {
+
+  StringRef Code = R"cpp(
+float bar(int i)
+{
+  return i;
+}
+
+void foo()
+{
+  bar(bar(3.0));
+}
+)cpp";
+
+  EXPECT_TRUE(matches(
+  Code,
+  traverse(ast_type_traits::TK_IgnoreImplicitCastsAndParentheses,
+   callExpr(has(callExpr(traverse(
+   ast_type_traits::TK_AsIs,
+   callExpr(has(implicitCastExpr(has(floatLiteral(;
+}
+
+TEST(Traversal, traverseMatcherThroughMemoization) {
+
+  StringRef Code = R"cpp(
+void foo()
+{
+  int i = 3.0;
+}
+  )cpp";
+
+  auto Matcher = varDecl(hasInitializer(floatLiteral()));
+
+  // Matchers such as hasDescendant memoize their result regarding AST
+  // nodes. In the matcher below, the first use of hasDescendant(Matcher)
+  // fails, and the use of it inside the traverse() matcher should pass
+  // causing the overall matcher to be a true match.
+  // This test verifies that the first false result is not re-used, which
+  // would cause the overall matcher to be incorrectly false.
+
+  EXPECT_TRUE(matches(
+  Code, functionDecl(anyOf(
+hasDescendant(Matcher),
+traverse(ast_type_traits::TK_IgnoreImplicitCastsAndParentheses,
+ functionDecl(hasDescendant(Matcher)));
+}
+
 TEST(IgnoringImpCasts, MatchesImpCasts) {
   // This test checks that ignoringImpCasts matches when implicit casts are
   // present and its inner matcher alone does not match.
Index: lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- lib/ASTMatchers/ASTMatchersInternal.cpp
+++ lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -211,10 +211,19 @@
 bool DynTypedMatcher::matches(const ast_type_traits::DynTypedNode ,
   ASTMatchFinder *Finder,
   BoundNodesTreeBuilder *Builder) const {
-  if (RestrictKind.isBaseOf(DynNode.getNodeKind()) &&
-  Implementation->dynMatches(DynNode, Finder, Builder)) {
+  auto PreviousTraversalKind = Finder->getASTContext().GetTraversalKind();
+  auto OptTK = Implementation->TraversalKind();
+  if (OptTK)
+Finder->getASTContext().SetTraversalKind(*OptTK);
+  auto N = Finder->getASTContext().TraverseIgnored(DynNode);
+  auto NodeKind = N.getNodeKind();
+
+  if (RestrictKind.isBaseOf(NodeKind) &&
+  Implementation->dynMatches(N, Finder, Builder)) {
+Finder->getASTContext().SetTraversalKind(PreviousTraversalKind);
 return true;
   }
+  Finder->getASTContext().SetTraversalKind(PreviousTraversalKind);
   // Delete all bindings when a matcher does not match.
   // This prevents unexpected exposure of bound nodes in unmatches
   // branches of the match tree.
@@ -225,8 +234,11 @@
 bool DynTypedMatcher::matchesNoKindCheck(
 const ast_type_traits::DynTypedNode , ASTMatchFinder *Finder,
 BoundNodesTreeBuilder *Builder) const {
-  assert(RestrictKind.isBaseOf(DynNode.getNodeKind()));
-  if (Implementation->dynMatches(DynNode, Finder, Builder)) {
+  auto N = Finder->getASTContext().TraverseIgnored(DynNode);
+  auto NodeKind = N.getNodeKind();
+
+  assert(RestrictKind.isBaseOf(NodeKind));
+  if (Implementation->dynMatches(N, Finder, Builder)) {
 return true;
   }
   // Delete all bindings when a matcher does not match.
Index: lib/ASTMatchers/ASTMatchFinder.cpp
===
--- lib/ASTMatchers/ASTMatchFinder.cpp
+++ lib/ASTMatchers/ASTMatchFinder.cpp
@@ -59,10 +59,12 @@
   DynTypedMatcher::MatcherIDType MatcherID;
   ast_type_traits::DynTypedNode Node;
   BoundNodesTreeBuilder BoundNodes;
+  ast_type_traits::TraversalKind Traversal = 

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

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D61827#1499347 , @hintonda wrote:

> 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?

Not in general, no. This is most certainly an improvement for normal C++ code.

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

For normal C++ - not that i know of, can't comment.

For loops that are OpenMP loops - yes, this will be a false-positive with 
erroneous, compilation-breaking, fixit.
(To be noted, the existing fix-its in this check already are that way.)

But as i said, this is only "for your information", only to be added to docs 
(if it isn't there already).
As i have said, this can not be reliably avoided, and i don't believe a partial 
avoidance will be good.

> Of that it ignores something it should fix?

That is called false-negative. No, this isn't the case here.


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] D61838: [Sema] Suppress additional warnings for C's zero initializer

[Alex James via Phabricator via cfe-commits]

al3xtjames created this revision.
al3xtjames added a reviewer: cfe-commits.
Herald added a project: clang.

r314499 relaxed some checks for assigning { 0 } to a structure for all
C standards, but it failed to handle structures with non-integer
subobjects. Relax -Wmissing-braces checks for such structures, and add
some additional tests.

This fixes PR39931.


Repository:
  rC Clang

https://reviews.llvm.org/D61838

Files:
  clang/lib/AST/Expr.cpp
  clang/test/Sema/zero-initializer.c


Index: clang/test/Sema/zero-initializer.c
===
--- clang/test/Sema/zero-initializer.c
+++ clang/test/Sema/zero-initializer.c
@@ -7,6 +7,8 @@
 struct B { struct A a; };
 struct C { struct B b; };
 struct D { struct C c; int n; };
+struct E { short e; };
+struct F { struct E e; int n; };
 
 int main(void)
 {
@@ -23,6 +25,9 @@
   struct C p = { 0 }; // no-warning
   struct C q = { 9 }; // warning suppressed for struct with single element
   struct D r = { 9 }; // expected-warning {{suggest braces around 
initialization of subobject}} expected-warning {{missing field 'n' initializer}}
+  struct F s = { 0 }; // no-warning
+  struct F t = { 9 }; // expected-warning {{suggest braces around 
initialization of subobject}} expected-warning {{missing field 'n' initializer}}
+
   f = (struct foo ) { 0 }; // no-warning
   g = (struct foo ) { 9 }; // expected-warning {{missing field 'y' 
initializer}}
   h = (struct foo ) { 9, 9 }; // no-warning
@@ -36,6 +41,8 @@
   p = (struct C) { 0 }; // no-warning
   q = (struct C) { 9 }; // warning suppressed for struct with single element
   r = (struct D) { 9 }; // expected-warning {{suggest braces around 
initialization of subobject}} expected-warning {{missing field 'n' initializer}}
+  s = (struct F) { 0 }; // no-warning
+  t = (struct F) { 9 }; // expected-warning {{suggest braces around 
initialization of subobject}} expected-warning {{missing field 'n' initializer}}
 
   return 0;
 }
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -2090,6 +2090,13 @@
   }
 
   const IntegerLiteral *Lit = dyn_cast(getInit(0));
+  if (!Lit) {
+const ImplicitCastExpr *Cast = dyn_cast(getInit(0));
+if (!Cast)
+  return false;
+Lit = dyn_cast(Cast->getSubExpr());
+  }
+
   return Lit && Lit->getValue() == 0;
 }
 


Index: clang/test/Sema/zero-initializer.c
===
--- clang/test/Sema/zero-initializer.c
+++ clang/test/Sema/zero-initializer.c
@@ -7,6 +7,8 @@
 struct B { struct A a; };
 struct C { struct B b; };
 struct D { struct C c; int n; };
+struct E { short e; };
+struct F { struct E e; int n; };
 
 int main(void)
 {
@@ -23,6 +25,9 @@
   struct C p = { 0 }; // no-warning
   struct C q = { 9 }; // warning suppressed for struct with single element
   struct D r = { 9 }; // expected-warning {{suggest braces around initialization of subobject}} expected-warning {{missing field 'n' initializer}}
+  struct F s = { 0 }; // no-warning
+  struct F t = { 9 }; // expected-warning {{suggest braces around initialization of subobject}} expected-warning {{missing field 'n' initializer}}
+
   f = (struct foo ) { 0 }; // no-warning
   g = (struct foo ) { 9 }; // expected-warning {{missing field 'y' initializer}}
   h = (struct foo ) { 9, 9 }; // no-warning
@@ -36,6 +41,8 @@
   p = (struct C) { 0 }; // no-warning
   q = (struct C) { 9 }; // warning suppressed for struct with single element
   r = (struct D) { 9 }; // expected-warning {{suggest braces around initialization of subobject}} expected-warning {{missing field 'n' initializer}}
+  s = (struct F) { 0 }; // no-warning
+  t = (struct F) { 9 }; // expected-warning {{suggest braces around initialization of subobject}} expected-warning {{missing field 'n' initializer}}
 
   return 0;
 }
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -2090,6 +2090,13 @@
   }
 
   const IntegerLiteral *Lit = dyn_cast(getInit(0));
+  if (!Lit) {
+const ImplicitCastExpr *Cast = dyn_cast(getInit(0));
+if (!Cast)
+  return false;
+Lit = dyn_cast(Cast->getSubExpr());
+  }
+
   return Lit && Lit->getValue() == 0;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61835: Extract ASTDumper to a header file

[Stephen Kelly via Phabricator via cfe-commits]

steveire updated this revision to Diff 199179.
steveire added a comment.

Format


Repository:
  rC Clang

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

https://reviews.llvm.org/D61835

Files:
  include/clang/AST/ASTDumper.h
  lib/AST/ASTDumper.cpp

Index: lib/AST/ASTDumper.cpp
===
--- lib/AST/ASTDumper.cpp
+++ lib/AST/ASTDumper.cpp
@@ -11,10 +11,9 @@
 //
 //===--===//
 
+#include "clang/AST/ASTDumper.h"
 #include "clang/AST/ASTContext.h"
-#include "clang/AST/ASTNodeTraverser.h"
 #include "clang/AST/DeclLookups.h"
-#include "clang/AST/TextNodeDumper.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/Module.h"
 #include "clang/Basic/SourceManager.h"
@@ -22,50 +21,6 @@
 using namespace clang;
 using namespace clang::comments;
 
-//===--===//
-// ASTDumper Visitor
-//===--===//
-
-namespace  {
-
-class ASTDumper : public ASTNodeTraverser {
-
-  TextNodeDumper NodeDumper;
-
-  raw_ostream 
-
-  const bool ShowColors;
-
-public:
-  ASTDumper(raw_ostream , const CommandTraits *Traits,
-const SourceManager *SM)
-  : ASTDumper(OS, Traits, SM, SM && SM->getDiagnostics().getShowColors()) {}
-
-  ASTDumper(raw_ostream , const CommandTraits *Traits,
-const SourceManager *SM, bool ShowColors)
-  : ASTDumper(OS, Traits, SM, ShowColors, LangOptions()) {}
-  ASTDumper(raw_ostream , const CommandTraits *Traits,
-const SourceManager *SM, bool ShowColors,
-const PrintingPolicy )
-  : NodeDumper(OS, ShowColors, SM, PrintPolicy, Traits), OS(OS),
-ShowColors(ShowColors) {}
-
-  TextNodeDumper () { return NodeDumper; }
-
-  void dumpLookups(const DeclContext *DC, bool DumpDecls);
-
-  template 
-  void dumpTemplateDeclSpecialization(const SpecializationDecl *D,
-  bool DumpExplicitInst, bool DumpRefOnly);
-  template 
-  void dumpTemplateDecl(const TemplateDecl *D, bool DumpExplicitInst);
-
-  void VisitFunctionTemplateDecl(const FunctionTemplateDecl *D);
-  void VisitClassTemplateDecl(const ClassTemplateDecl *D);
-  void VisitVarTemplateDecl(const VarTemplateDecl *D);
-};
-} // namespace
-
 void ASTDumper::dumpLookups(const DeclContext *DC, bool DumpDecls) {
   NodeDumper.AddChild([=] {
 OS << "StoredDeclsMap ";
Index: include/clang/AST/ASTDumper.h
===
--- /dev/null
+++ include/clang/AST/ASTDumper.h
@@ -0,0 +1,56 @@
+//===--- ASTDumper.h - Dumping implementation for ASTs ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_AST_ASTDUMPER_H
+#define LLVM_CLANG_AST_ASTDUMPER_H
+
+#include "clang/AST/ASTNodeTraverser.h"
+#include "clang/AST/TextNodeDumper.h"
+
+namespace clang {
+
+class ASTDumper : public ASTNodeTraverser {
+
+  TextNodeDumper NodeDumper;
+
+  raw_ostream 
+
+  const bool ShowColors;
+
+public:
+  ASTDumper(raw_ostream , const comments::CommandTraits *Traits,
+const SourceManager *SM)
+  : ASTDumper(OS, Traits, SM, SM && SM->getDiagnostics().getShowColors()) {}
+
+  ASTDumper(raw_ostream , const comments::CommandTraits *Traits,
+const SourceManager *SM, bool ShowColors)
+  : ASTDumper(OS, Traits, SM, ShowColors, LangOptions()) {}
+  ASTDumper(raw_ostream , const comments::CommandTraits *Traits,
+const SourceManager *SM, bool ShowColors,
+const PrintingPolicy )
+  : NodeDumper(OS, ShowColors, SM, PrintPolicy, Traits), OS(OS),
+ShowColors(ShowColors) {}
+
+  TextNodeDumper () { return NodeDumper; }
+
+  void dumpLookups(const DeclContext *DC, bool DumpDecls);
+
+  template 
+  void dumpTemplateDeclSpecialization(const SpecializationDecl *D,
+  bool DumpExplicitInst, bool DumpRefOnly);
+  template 
+  void dumpTemplateDecl(const TemplateDecl *D, bool DumpExplicitInst);
+
+  void VisitFunctionTemplateDecl(const FunctionTemplateDecl *D);
+  void VisitClassTemplateDecl(const ClassTemplateDecl *D);
+  void VisitVarTemplateDecl(const VarTemplateDecl *D);
+};
+
+} // namespace clang
+
+#endif
___
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

[Stephen Kelly via Phabricator via cfe-commits]

steveire added inline comments.



Comment at: include/clang/AST/DeclBase.h:1139
+  void dump(raw_ostream , bool Deserialize = false,
+ASTDumpOutputFormat OutputFormat = ADOF_Default) const;
 

aaron.ballman wrote:
> steveire wrote:
> > I think we've talked about this before, but I don't think growing 
> > interfaces like this is the best way forward. An enum is a less-good 
> > replacement for an object (ie making the user of the API responsible for 
> > creating the dumper they want to use).
> > 
> > I think that could be made more convenient in the future. What do you think?
> I think that may be a good future improvement, yes.
> 
> One thing to take on balance when considering this for the future is that 
> `dump()` can be called from within a debugger and that's a bit harder to 
> accomplish with an object interface. I'm not certain that will be a big issue 
> for my use case, but it may matter to some folks.
I don't think "it may matter to some folks." is a good guideline to use when 
designing an API. I have a patch which moves ASTDumper to its own header which 
I have just uploaded here: https://reviews.llvm.org/D61835

If additional args to `dump()` are mainly for debugging, then adding new args 
which are not for debugging doesn't seem right. 


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] D61836: Move TraversalKind enum to ast_type_traits

[Stephen Kelly via Phabricator via cfe-commits]

steveire updated this revision to Diff 199180.
steveire added a comment.

Format


Repository:
  rC Clang

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

https://reviews.llvm.org/D61836

Files:
  include/clang/AST/ASTTypeTraits.h
  include/clang/ASTMatchers/ASTMatchersInternal.h
  lib/ASTMatchers/ASTMatchFinder.cpp
  unittests/ASTMatchers/ASTMatchersInternalTest.cpp

Index: unittests/ASTMatchers/ASTMatchersInternalTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersInternalTest.cpp
+++ unittests/ASTMatchers/ASTMatchersInternalTest.cpp
@@ -76,7 +76,7 @@
   internal::Matcher, AMatcher) {
   return Finder->matchesChildOf(
   Node, AMatcher, Builder,
-  ASTMatchFinder::TK_IgnoreImplicitCastsAndParentheses,
+  ast_type_traits::TraversalKind::TK_IgnoreImplicitCastsAndParentheses,
   ASTMatchFinder::BK_First);
 }
 
Index: lib/ASTMatchers/ASTMatchFinder.cpp
===
--- lib/ASTMatchers/ASTMatchFinder.cpp
+++ lib/ASTMatchers/ASTMatchFinder.cpp
@@ -83,20 +83,12 @@
   // descendants of a traversed node. max_depth is the maximum depth
   // to traverse: use 1 for matching the children and INT_MAX for
   // matching the descendants.
-  MatchChildASTVisitor(const DynTypedMatcher *Matcher,
-   ASTMatchFinder *Finder,
-   BoundNodesTreeBuilder *Builder,
-   int MaxDepth,
-   ASTMatchFinder::TraversalKind Traversal,
+  MatchChildASTVisitor(const DynTypedMatcher *Matcher, ASTMatchFinder *Finder,
+   BoundNodesTreeBuilder *Builder, int MaxDepth,
+   ast_type_traits::TraversalKind Traversal,
ASTMatchFinder::BindKind Bind)
-  : Matcher(Matcher),
-Finder(Finder),
-Builder(Builder),
-CurrentDepth(0),
-MaxDepth(MaxDepth),
-Traversal(Traversal),
-Bind(Bind),
-Matches(false) {}
+  : Matcher(Matcher), Finder(Finder), Builder(Builder), CurrentDepth(0),
+MaxDepth(MaxDepth), Traversal(Traversal), Bind(Bind), Matches(false) {}
 
   // Returns true if a match is found in the subtree rooted at the
   // given AST node. This is done via a set of mutually recursive
@@ -151,7 +143,8 @@
 
 ScopedIncrement ScopedDepth();
 Stmt *StmtToTraverse = StmtNode;
-if (Traversal == ASTMatchFinder::TK_IgnoreImplicitCastsAndParentheses) {
+if (Traversal ==
+ast_type_traits::TraversalKind::TK_IgnoreImplicitCastsAndParentheses) {
   if (Expr *ExprNode = dyn_cast_or_null(StmtNode))
 StmtToTraverse = ExprNode->IgnoreParenImpCasts();
 }
@@ -299,7 +292,7 @@
   BoundNodesTreeBuilder ResultBindings;
   int CurrentDepth;
   const int MaxDepth;
-  const ASTMatchFinder::TraversalKind Traversal;
+  const ast_type_traits::TraversalKind Traversal;
   const ASTMatchFinder::BindKind Bind;
   bool Matches;
 };
@@ -393,7 +386,8 @@
   bool memoizedMatchesRecursively(const ast_type_traits::DynTypedNode ,
   const DynTypedMatcher ,
   BoundNodesTreeBuilder *Builder, int MaxDepth,
-  TraversalKind Traversal, BindKind Bind) {
+  ast_type_traits::TraversalKind Traversal,
+  BindKind Bind) {
 // For AST-nodes that don't have an identity, we can't memoize.
 if (!Node.getMemoizationData() || !Builder->isComparable())
   return matchesRecursively(Node, Matcher, Builder, MaxDepth, Traversal,
@@ -427,7 +421,8 @@
   bool matchesRecursively(const ast_type_traits::DynTypedNode ,
   const DynTypedMatcher ,
   BoundNodesTreeBuilder *Builder, int MaxDepth,
-  TraversalKind Traversal, BindKind Bind) {
+  ast_type_traits::TraversalKind Traversal,
+  BindKind Bind) {
 MatchChildASTVisitor Visitor(
   , this, Builder, MaxDepth, Traversal, Bind);
 return Visitor.findMatch(Node);
@@ -441,7 +436,7 @@
   bool matchesChildOf(const ast_type_traits::DynTypedNode ,
   const DynTypedMatcher ,
   BoundNodesTreeBuilder *Builder,
-  TraversalKind Traversal,
+  ast_type_traits::TraversalKind Traversal,
   BindKind Bind) override {
 if (ResultCache.size() > MaxMemoizationEntries)
   ResultCache.clear();
@@ -456,7 +451,8 @@
 if (ResultCache.size() > MaxMemoizationEntries)
   ResultCache.clear();
 return memoizedMatchesRecursively(Node, Matcher, Builder, INT_MAX,
-  TK_AsIs, Bind);
+  ast_type_traits::TraversalKind::TK_AsIs,
+  Bind);
   }
   // Implements 

[PATCH] D61834: Add a Visit overload for DynTypedNode to ASTNodeTraverser

[Stephen Kelly via Phabricator via cfe-commits]

steveire updated this revision to Diff 199178.
steveire added a comment.

Format


Repository:
  rC Clang

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

https://reviews.llvm.org/D61834

Files:
  include/clang/AST/ASTNodeTraverser.h


Index: include/clang/AST/ASTNodeTraverser.h
===
--- include/clang/AST/ASTNodeTraverser.h
+++ include/clang/AST/ASTNodeTraverser.h
@@ -205,6 +205,27 @@
 });
   }
 
+  void Visit(const ast_type_traits::DynTypedNode ) {
+if (const auto *D = N.get())
+  Visit(D);
+else if (const auto *S = N.get())
+  Visit(S);
+else if (const auto *QT = N.get())
+  Visit(*QT);
+else if (const auto *T = N.get())
+  Visit(T);
+else if (const auto *A = N.get())
+  Visit(A);
+else if (const auto *C = N.get())
+  Visit(C);
+else if (const auto *C = N.get())
+  Visit(C);
+else if (const auto *C = N.get())
+  Visit(C, C);
+else if (const auto *T = N.get())
+  Visit(*T);
+  }
+
   void dumpDeclContext(const DeclContext *DC) {
 if (!DC)
   return;


Index: include/clang/AST/ASTNodeTraverser.h
===
--- include/clang/AST/ASTNodeTraverser.h
+++ include/clang/AST/ASTNodeTraverser.h
@@ -205,6 +205,27 @@
 });
   }
 
+  void Visit(const ast_type_traits::DynTypedNode ) {
+if (const auto *D = N.get())
+  Visit(D);
+else if (const auto *S = N.get())
+  Visit(S);
+else if (const auto *QT = N.get())
+  Visit(*QT);
+else if (const auto *T = N.get())
+  Visit(T);
+else if (const auto *A = N.get())
+  Visit(A);
+else if (const auto *C = N.get())
+  Visit(C);
+else if (const auto *C = N.get())
+  Visit(C);
+else if (const auto *C = N.get())
+  Visit(C, C);
+else if (const auto *T = N.get())
+  Visit(*T);
+  }
+
   void dumpDeclContext(const DeclContext *DC) {
 if (!DC)
   return;
___
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] D61837: Make it possible control matcher traversal kind with ASTContext

[Stephen Kelly via Phabricator via cfe-commits]

steveire updated this revision to Diff 199181.
steveire added a comment.

Format


Repository:
  rC Clang

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

https://reviews.llvm.org/D61837

Files:
  include/clang/AST/ASTContext.h
  include/clang/AST/ASTNodeTraverser.h
  include/clang/ASTMatchers/ASTMatchers.h
  include/clang/ASTMatchers/ASTMatchersInternal.h
  lib/AST/ASTContext.cpp
  lib/ASTMatchers/ASTMatchFinder.cpp
  lib/ASTMatchers/ASTMatchersInternal.cpp
  unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -1510,6 +1510,72 @@
   notMatches("class C {}; C a = C();", varDecl(has(cxxConstructExpr();
 }
 
+TEST(Traversal, traverseMatcher) {
+
+  StringRef VarDeclCode = R"cpp(
+void foo()
+{
+  int i = 3.0;
+}
+)cpp";
+
+  auto Matcher = varDecl(hasInitializer(floatLiteral()));
+
+  EXPECT_TRUE(
+  notMatches(VarDeclCode, traverse(ast_type_traits::TK_AsIs, Matcher)));
+  EXPECT_TRUE(
+  matches(VarDeclCode,
+  traverse(ast_type_traits::TK_IgnoreImplicitCastsAndParentheses,
+   Matcher)));
+}
+
+TEST(Traversal, traverseMatcherNesting) {
+
+  StringRef Code = R"cpp(
+float bar(int i)
+{
+  return i;
+}
+
+void foo()
+{
+  bar(bar(3.0));
+}
+)cpp";
+
+  EXPECT_TRUE(matches(
+  Code,
+  traverse(ast_type_traits::TK_IgnoreImplicitCastsAndParentheses,
+   callExpr(has(callExpr(traverse(
+   ast_type_traits::TK_AsIs,
+   callExpr(has(implicitCastExpr(has(floatLiteral(;
+}
+
+TEST(Traversal, traverseMatcherThroughMemoization) {
+
+  StringRef Code = R"cpp(
+void foo()
+{
+  int i = 3.0;
+}
+  )cpp";
+
+  auto Matcher = varDecl(hasInitializer(floatLiteral()));
+
+  // Matchers such as hasDescendant memoize their result regarding AST
+  // nodes. In the matcher below, the first use of hasDescendant(Matcher)
+  // fails, and the use of it inside the traverse() matcher should pass
+  // causing the overall matcher to be a true match.
+  // This test verifies that the first false result is not re-used, which
+  // would cause the overall matcher to be incorrectly false.
+
+  EXPECT_TRUE(matches(
+  Code, functionDecl(anyOf(
+hasDescendant(Matcher),
+traverse(ast_type_traits::TK_IgnoreImplicitCastsAndParentheses,
+ functionDecl(hasDescendant(Matcher)));
+}
+
 TEST(IgnoringImpCasts, MatchesImpCasts) {
   // This test checks that ignoringImpCasts matches when implicit casts are
   // present and its inner matcher alone does not match.
Index: lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- lib/ASTMatchers/ASTMatchersInternal.cpp
+++ lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -211,10 +211,17 @@
 bool DynTypedMatcher::matches(const ast_type_traits::DynTypedNode ,
   ASTMatchFinder *Finder,
   BoundNodesTreeBuilder *Builder) const {
-  if (RestrictKind.isBaseOf(DynNode.getNodeKind()) &&
-  Implementation->dynMatches(DynNode, Finder, Builder)) {
+  auto PreviousTraversalKind = Finder->getASTContext().GetTraversalKind();
+  Finder->getASTContext().SetTraversalKind(Implementation->TraversalKind());
+  auto N = Finder->getASTContext().TraverseIgnored(DynNode);
+  auto NodeKind = N.getNodeKind();
+
+  if (RestrictKind.isBaseOf(NodeKind) &&
+  Implementation->dynMatches(N, Finder, Builder)) {
+Finder->getASTContext().SetTraversalKind(PreviousTraversalKind);
 return true;
   }
+  Finder->getASTContext().SetTraversalKind(PreviousTraversalKind);
   // Delete all bindings when a matcher does not match.
   // This prevents unexpected exposure of bound nodes in unmatches
   // branches of the match tree.
@@ -225,8 +232,11 @@
 bool DynTypedMatcher::matchesNoKindCheck(
 const ast_type_traits::DynTypedNode , ASTMatchFinder *Finder,
 BoundNodesTreeBuilder *Builder) const {
-  assert(RestrictKind.isBaseOf(DynNode.getNodeKind()));
-  if (Implementation->dynMatches(DynNode, Finder, Builder)) {
+  auto N = Finder->getASTContext().TraverseIgnored(DynNode);
+  auto NodeKind = N.getNodeKind();
+
+  assert(RestrictKind.isBaseOf(NodeKind));
+  if (Implementation->dynMatches(N, Finder, Builder)) {
 return true;
   }
   // Delete all bindings when a matcher does not match.
Index: lib/ASTMatchers/ASTMatchFinder.cpp
===
--- lib/ASTMatchers/ASTMatchFinder.cpp
+++ lib/ASTMatchers/ASTMatchFinder.cpp
@@ -59,10 +59,12 @@
   DynTypedMatcher::MatcherIDType MatcherID;
   ast_type_traits::DynTypedNode Node;
   BoundNodesTreeBuilder BoundNodes;
+  ast_type_traits::TraversalKind Traversal = ast_type_traits::TK_AsIs;
 
   bool 

[PATCH] D61837: Make it possible control matcher traversal kind with ASTContext

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

@klimek This includes a test for the memoization case you were interested in at 
EuroLLVM.

In this iteration of the API, the traversal kind is changed by using a new 
`traverse` matcher, which gives the user control over whether 
invisible/implicit nodes are skipped or not.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61837



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

[PATCH] D61837: Make it possible control matcher traversal kind with ASTContext

[Stephen Kelly via Phabricator via cfe-commits]

steveire created this revision.
steveire added reviewers: klimek, aaron.ballman.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This will eventually allow traversal of an AST while ignoring invisible
AST nodes.  Currently it depends on the available enum values for
TraversalKinds.  That can be extended to ignore all invisible nodes in
the future.


Repository:
  rC Clang

https://reviews.llvm.org/D61837

Files:
  include/clang/AST/ASTContext.h
  include/clang/AST/ASTNodeTraverser.h
  include/clang/ASTMatchers/ASTMatchers.h
  include/clang/ASTMatchers/ASTMatchersInternal.h
  lib/AST/ASTContext.cpp
  lib/ASTMatchers/ASTMatchFinder.cpp
  lib/ASTMatchers/ASTMatchersInternal.cpp
  unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -1510,6 +1510,96 @@
   notMatches("class C {}; C a = C();", varDecl(has(cxxConstructExpr();
 }
 
+TEST(Traversal, traverseMatcher) {
+
+  StringRef VarDeclCode = R"cpp(
+void foo()
+{
+  int i = 3.0;
+}
+)cpp";
+
+  auto Matcher = varDecl(hasInitializer(floatLiteral()));
+
+  EXPECT_TRUE(notMatches(
+VarDeclCode,
+traverse(ast_type_traits::TK_AsIs,
+  Matcher
+  )
+  ));
+  EXPECT_TRUE(matches(
+VarDeclCode,
+traverse(ast_type_traits::TK_IgnoreImplicitCastsAndParentheses,
+  Matcher
+  )
+  ));
+}
+
+TEST(Traversal, traverseMatcherNesting) {
+
+  StringRef Code = R"cpp(
+float bar(int i)
+{
+  return i;
+}
+
+void foo()
+{
+  bar(bar(3.0));
+}
+)cpp";
+
+  EXPECT_TRUE(matches(
+Code,
+traverse(ast_type_traits::TK_IgnoreImplicitCastsAndParentheses,
+  callExpr(has(
+callExpr(
+  traverse(ast_type_traits::TK_AsIs,
+callExpr(has(implicitCastExpr(has(floatLiteral()
+)
+  )
+))
+  )
+  ));
+}
+
+TEST(Traversal, traverseMatcherThroughMemoization) {
+
+  StringRef Code = R"cpp(
+void foo()
+{
+  int i = 3.0;
+}
+  )cpp";
+
+  auto Matcher = varDecl(hasInitializer(floatLiteral()));
+
+  // Matchers such as hasDescendant memoize their result regarding AST
+  // nodes. In the matcher below, the first use of hasDescendant(Matcher)
+  // fails, and the use of it inside the traverse() matcher should pass
+  // causing the overall matcher to be a true match.
+  // This test verifies that the first false result is not re-used, which
+  // would cause the overall matcher to be incorrectly false.
+
+  EXPECT_TRUE(
+matches(
+Code,
+functionDecl(anyOf(
+  hasDescendant(
+Matcher
+),
+  traverse(ast_type_traits::TK_IgnoreImplicitCastsAndParentheses,
+functionDecl(
+  hasDescendant(
+Matcher
+)
+  )
+)
+  ))
+)
+  );
+}
+
 TEST(IgnoringImpCasts, MatchesImpCasts) {
   // This test checks that ignoringImpCasts matches when implicit casts are
   // present and its inner matcher alone does not match.
Index: lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- lib/ASTMatchers/ASTMatchersInternal.cpp
+++ lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -211,8 +211,11 @@
 bool DynTypedMatcher::matches(const ast_type_traits::DynTypedNode ,
   ASTMatchFinder *Finder,
   BoundNodesTreeBuilder *Builder) const {
-  if (RestrictKind.isBaseOf(DynNode.getNodeKind()) &&
-  Implementation->dynMatches(DynNode, Finder, Builder)) {
+  auto N = Finder->getASTContext().TraverseIgnored(DynNode);
+  auto NodeKind = N.getNodeKind();
+
+  if (RestrictKind.isBaseOf(NodeKind) &&
+  Implementation->dynMatches(N, Finder, Builder)) {
 return true;
   }
   // Delete all bindings when a matcher does not match.
@@ -225,8 +228,11 @@
 bool DynTypedMatcher::matchesNoKindCheck(
 const ast_type_traits::DynTypedNode , ASTMatchFinder *Finder,
 BoundNodesTreeBuilder *Builder) const {
-  assert(RestrictKind.isBaseOf(DynNode.getNodeKind()));
-  if (Implementation->dynMatches(DynNode, Finder, Builder)) {
+  auto N = Finder->getASTContext().TraverseIgnored(DynNode);
+  auto NodeKind = N.getNodeKind();
+
+  assert(RestrictKind.isBaseOf(NodeKind));
+  if (Implementation->dynMatches(N, Finder, Builder)) {
 return true;
   }
   // Delete all bindings when a matcher does not match.
Index: lib/ASTMatchers/ASTMatchFinder.cpp
===
--- lib/ASTMatchers/ASTMatchFinder.cpp
+++ lib/ASTMatchers/ASTMatchFinder.cpp
@@ -59,10 +59,11 @@
   DynTypedMatcher::MatcherIDType MatcherID;
   ast_type_traits::DynTypedNode Node;
   BoundNodesTreeBuilder BoundNodes;
+  ast_type_traits::TraversalKind Traversal = ast_type_traits::TK_AsIs;
 
   bool operator<(const MatchKey ) const {
-return std::tie(MatcherID, 

[PATCH] D61836: Move TraversalKind enum to ast_type_traits

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

This is part of the work I demo'd at EuroLLVM for ignoring invisible AST nodes 
during AST Matching and dumping: http://ce.steveire.com/z/lHYwEH


Repository:
  rC Clang

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

https://reviews.llvm.org/D61836



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

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

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D61827#1499333 , @hintonda wrote:

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


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)


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] D61836: Move TraversalKind enum to ast_type_traits

[Stephen Kelly via Phabricator via cfe-commits]

steveire created this revision.
steveire added reviewers: klimek, aaron.ballman.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Make it usable outside of ASTMatchFinder.  This will make it possible to
use this enum to control whether certain implicit nodes are skipped
while AST dumping for example.


Repository:
  rC Clang

https://reviews.llvm.org/D61836

Files:
  include/clang/AST/ASTTypeTraits.h
  include/clang/ASTMatchers/ASTMatchersInternal.h
  lib/ASTMatchers/ASTMatchFinder.cpp
  unittests/ASTMatchers/ASTMatchersInternalTest.cpp

Index: unittests/ASTMatchers/ASTMatchersInternalTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersInternalTest.cpp
+++ unittests/ASTMatchers/ASTMatchersInternalTest.cpp
@@ -76,7 +76,7 @@
   internal::Matcher, AMatcher) {
   return Finder->matchesChildOf(
   Node, AMatcher, Builder,
-  ASTMatchFinder::TK_IgnoreImplicitCastsAndParentheses,
+  ast_type_traits::TraversalKind::TK_IgnoreImplicitCastsAndParentheses,
   ASTMatchFinder::BK_First);
 }
 
Index: lib/ASTMatchers/ASTMatchFinder.cpp
===
--- lib/ASTMatchers/ASTMatchFinder.cpp
+++ lib/ASTMatchers/ASTMatchFinder.cpp
@@ -87,7 +87,7 @@
ASTMatchFinder *Finder,
BoundNodesTreeBuilder *Builder,
int MaxDepth,
-   ASTMatchFinder::TraversalKind Traversal,
+   ast_type_traits::TraversalKind Traversal,
ASTMatchFinder::BindKind Bind)
   : Matcher(Matcher),
 Finder(Finder),
@@ -151,7 +151,7 @@
 
 ScopedIncrement ScopedDepth();
 Stmt *StmtToTraverse = StmtNode;
-if (Traversal == ASTMatchFinder::TK_IgnoreImplicitCastsAndParentheses) {
+if (Traversal == ast_type_traits::TraversalKind::TK_IgnoreImplicitCastsAndParentheses) {
   if (Expr *ExprNode = dyn_cast_or_null(StmtNode))
 StmtToTraverse = ExprNode->IgnoreParenImpCasts();
 }
@@ -299,7 +299,7 @@
   BoundNodesTreeBuilder ResultBindings;
   int CurrentDepth;
   const int MaxDepth;
-  const ASTMatchFinder::TraversalKind Traversal;
+  const ast_type_traits::TraversalKind Traversal;
   const ASTMatchFinder::BindKind Bind;
   bool Matches;
 };
@@ -393,7 +393,7 @@
   bool memoizedMatchesRecursively(const ast_type_traits::DynTypedNode ,
   const DynTypedMatcher ,
   BoundNodesTreeBuilder *Builder, int MaxDepth,
-  TraversalKind Traversal, BindKind Bind) {
+  ast_type_traits::TraversalKind Traversal, BindKind Bind) {
 // For AST-nodes that don't have an identity, we can't memoize.
 if (!Node.getMemoizationData() || !Builder->isComparable())
   return matchesRecursively(Node, Matcher, Builder, MaxDepth, Traversal,
@@ -427,7 +427,7 @@
   bool matchesRecursively(const ast_type_traits::DynTypedNode ,
   const DynTypedMatcher ,
   BoundNodesTreeBuilder *Builder, int MaxDepth,
-  TraversalKind Traversal, BindKind Bind) {
+  ast_type_traits::TraversalKind Traversal, BindKind Bind) {
 MatchChildASTVisitor Visitor(
   , this, Builder, MaxDepth, Traversal, Bind);
 return Visitor.findMatch(Node);
@@ -441,7 +441,7 @@
   bool matchesChildOf(const ast_type_traits::DynTypedNode ,
   const DynTypedMatcher ,
   BoundNodesTreeBuilder *Builder,
-  TraversalKind Traversal,
+  ast_type_traits::TraversalKind Traversal,
   BindKind Bind) override {
 if (ResultCache.size() > MaxMemoizationEntries)
   ResultCache.clear();
@@ -456,7 +456,7 @@
 if (ResultCache.size() > MaxMemoizationEntries)
   ResultCache.clear();
 return memoizedMatchesRecursively(Node, Matcher, Builder, INT_MAX,
-  TK_AsIs, Bind);
+  ast_type_traits::TraversalKind::TK_AsIs, Bind);
   }
   // Implements ASTMatchFinder::matchesAncestorOf.
   bool matchesAncestorOf(const ast_type_traits::DynTypedNode ,
Index: include/clang/ASTMatchers/ASTMatchersInternal.h
===
--- include/clang/ASTMatchers/ASTMatchersInternal.h
+++ include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -950,15 +950,6 @@
 /// all nodes, as all nodes have ancestors.
 class ASTMatchFinder {
 public:
-  /// Defines how we descend a level in the AST when we pass
-  /// through expressions.
-  enum TraversalKind {
-/// Will traverse any child nodes.
-TK_AsIs,
-
-/// Will not traverse implicit casts and parentheses.
-TK_IgnoreImplicitCastsAndParentheses
-  };
 
   /// Defines how bindings are processed on recursive matches.
  

[PATCH] D61835: Extract ASTDumper to a header file

[Stephen Kelly via Phabricator via cfe-commits]

steveire created this revision.
steveire added a reviewer: aaron.ballman.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This class has member APIs which are useful to clients.  Make it
possible to use those APIs without adding them to dump() member
functions.  Doing so does not scale.  The optional arguments to dump()
should be designed to be useful in a debugging context.


Repository:
  rC Clang

https://reviews.llvm.org/D61835

Files:
  include/clang/AST/ASTDumper.h
  lib/AST/ASTDumper.cpp

Index: lib/AST/ASTDumper.cpp
===
--- lib/AST/ASTDumper.cpp
+++ lib/AST/ASTDumper.cpp
@@ -11,10 +11,9 @@
 //
 //===--===//
 
+#include "clang/AST/ASTDumper.h"
 #include "clang/AST/ASTContext.h"
-#include "clang/AST/ASTNodeTraverser.h"
 #include "clang/AST/DeclLookups.h"
-#include "clang/AST/TextNodeDumper.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/Module.h"
 #include "clang/Basic/SourceManager.h"
@@ -22,50 +21,6 @@
 using namespace clang;
 using namespace clang::comments;
 
-//===--===//
-// ASTDumper Visitor
-//===--===//
-
-namespace  {
-
-class ASTDumper : public ASTNodeTraverser {
-
-  TextNodeDumper NodeDumper;
-
-  raw_ostream 
-
-  const bool ShowColors;
-
-public:
-  ASTDumper(raw_ostream , const CommandTraits *Traits,
-const SourceManager *SM)
-  : ASTDumper(OS, Traits, SM, SM && SM->getDiagnostics().getShowColors()) {}
-
-  ASTDumper(raw_ostream , const CommandTraits *Traits,
-const SourceManager *SM, bool ShowColors)
-  : ASTDumper(OS, Traits, SM, ShowColors, LangOptions()) {}
-  ASTDumper(raw_ostream , const CommandTraits *Traits,
-const SourceManager *SM, bool ShowColors,
-const PrintingPolicy )
-  : NodeDumper(OS, ShowColors, SM, PrintPolicy, Traits), OS(OS),
-ShowColors(ShowColors) {}
-
-  TextNodeDumper () { return NodeDumper; }
-
-  void dumpLookups(const DeclContext *DC, bool DumpDecls);
-
-  template 
-  void dumpTemplateDeclSpecialization(const SpecializationDecl *D,
-  bool DumpExplicitInst, bool DumpRefOnly);
-  template 
-  void dumpTemplateDecl(const TemplateDecl *D, bool DumpExplicitInst);
-
-  void VisitFunctionTemplateDecl(const FunctionTemplateDecl *D);
-  void VisitClassTemplateDecl(const ClassTemplateDecl *D);
-  void VisitVarTemplateDecl(const VarTemplateDecl *D);
-};
-} // namespace
-
 void ASTDumper::dumpLookups(const DeclContext *DC, bool DumpDecls) {
   NodeDumper.AddChild([=] {
 OS << "StoredDeclsMap ";
Index: include/clang/AST/ASTDumper.h
===
--- /dev/null
+++ include/clang/AST/ASTDumper.h
@@ -0,0 +1,56 @@
+//===--- ASTDumper.h - Dumping implementation for ASTs ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_AST_ASTDUMPER_H
+#define LLVM_CLANG_AST_ASTDUMPER_H
+
+#include "clang/AST/ASTNodeTraverser.h"
+#include "clang/AST/TextNodeDumper.h"
+
+namespace clang {
+
+class ASTDumper : public ASTNodeTraverser {
+
+  TextNodeDumper NodeDumper;
+
+  raw_ostream 
+
+  const bool ShowColors;
+
+public:
+  ASTDumper(raw_ostream , const comments::CommandTraits *Traits,
+const SourceManager *SM)
+  : ASTDumper(OS, Traits, SM, SM && SM->getDiagnostics().getShowColors()) {}
+
+  ASTDumper(raw_ostream , const comments::CommandTraits *Traits,
+const SourceManager *SM, bool ShowColors)
+  : ASTDumper(OS, Traits, SM, ShowColors, LangOptions()) {}
+  ASTDumper(raw_ostream , const comments::CommandTraits *Traits,
+const SourceManager *SM, bool ShowColors,
+const PrintingPolicy )
+  : NodeDumper(OS, ShowColors, SM, PrintPolicy, Traits), OS(OS),
+ShowColors(ShowColors) {}
+
+  TextNodeDumper () { return NodeDumper; }
+
+  void dumpLookups(const DeclContext *DC, bool DumpDecls);
+
+  template 
+  void dumpTemplateDeclSpecialization(const SpecializationDecl *D,
+  bool DumpExplicitInst, bool DumpRefOnly);
+  template 
+  void dumpTemplateDecl(const TemplateDecl *D, bool DumpExplicitInst);
+
+  void VisitFunctionTemplateDecl(const FunctionTemplateDecl *D);
+  void VisitClassTemplateDecl(const ClassTemplateDecl *D);
+  void VisitVarTemplateDecl(const VarTemplateDecl *D);
+};
+
+} // clang
+
+#endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D61834: Add a Visit overload for DynTypedNode to ASTNodeTraverser

[Stephen Kelly via Phabricator via cfe-commits]

steveire created this revision.
steveire added a reviewer: aaron.ballman.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

https://reviews.llvm.org/D61834

Files:
  include/clang/AST/ASTNodeTraverser.h


Index: include/clang/AST/ASTNodeTraverser.h
===
--- include/clang/AST/ASTNodeTraverser.h
+++ include/clang/AST/ASTNodeTraverser.h
@@ -205,6 +205,27 @@
 });
   }
 
+  void Visit(const ast_type_traits::DynTypedNode& N) {
+if (const auto *D = N.get())
+  Visit(D);
+else if (const auto *S = N.get())
+  Visit(S);
+else if (const auto* QT = N.get())
+  Visit(*QT);
+else if (const auto *T = N.get())
+  Visit(T);
+else if (const auto *A = N.get())
+  Visit(A);
+else if (const auto *C = N.get())
+  Visit(C);
+else if (const auto *C = N.get())
+  Visit(C);
+else if (const auto *C = N.get())
+  Visit(C, C);
+else if (const auto* T = N.get())
+  Visit(*T);
+  }
+
   void dumpDeclContext(const DeclContext *DC) {
 if (!DC)
   return;


Index: include/clang/AST/ASTNodeTraverser.h
===
--- include/clang/AST/ASTNodeTraverser.h
+++ include/clang/AST/ASTNodeTraverser.h
@@ -205,6 +205,27 @@
 });
   }
 
+  void Visit(const ast_type_traits::DynTypedNode& N) {
+if (const auto *D = N.get())
+  Visit(D);
+else if (const auto *S = N.get())
+  Visit(S);
+else if (const auto* QT = N.get())
+  Visit(*QT);
+else if (const auto *T = N.get())
+  Visit(T);
+else if (const auto *A = N.get())
+  Visit(A);
+else if (const auto *C = N.get())
+  Visit(C);
+else if (const auto *C = N.get())
+  Visit(C);
+else if (const auto *C = N.get())
+  Visit(C, C);
+else if (const auto* T = N.get())
+  Visit(*T);
+  }
+
   void dumpDeclContext(const DeclContext *DC) {
 if (!DC)
   return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D61827#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?

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?
> > > >
> > >
> >
>




> 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] D51329: [Attribute/Diagnostics] Print macro if definition is an attribute declaration

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

This caused a regression when including mingw-w64 headers (in a case where they 
happen to include redundant/duplicate attribute specifications), triggering 
asserts. See https://bugs.llvm.org/show_bug.cgi?id=41852 for details.


Repository:
  rC Clang

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

https://reviews.llvm.org/D51329



___
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

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri 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(,
>


Yes and no.
This **will** prevent the false-positive, but **only if** the OpenMP is 
**enabled** (`-fopenmp`).
If OpenMP is **not enabled**, then that **won't work**, because there won't be 
anything about OpenMP in AST.
I semi-strongly believe it will be less confusing to only document this 
false-positive (in check's docs),
instead of trying to prevent it, and reliably failing in half of the cases.


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

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

torbjoernk added a comment.

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(,


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

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

torbjoernk 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);

hintonda wrote:
> 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  }
> 
This seems weird and I reckon my setup is broken as also a bunch of other lit 
tests (make check-clang-tools) fail.

modernize-loop-convert-basic fails once I change this to the following:

```
  dependent V;
  for (dependent::const_iterator It = V.begin(), E = V.end();
   It != E; ++It) {
printf("Fibonacci number is %d\n", *It);
  }
  // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
  // CHECK-FIXES: for (int It : V)
  // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", It);

  for (dependent::const_iterator It(V.begin()), E = V.end();
   It != E; ++It) {
printf("Fibonacci number is %d\n", *It);
  }
  // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
  // CHECK-FIXES: for (int It : V)
  // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", It);
```

However, copy outsite of llvm-lit in a minimal example, it is detected 
and fixed as expected.

Is it possible that llvm-lit picks up my (older) globally available clang-tidy?


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] D61804: Support building shared and static libraries simultaneously

[Wink Saville via Phabricator via cfe-commits]

winksaville updated this revision to Diff 199172.
winksaville added a comment.

Change default to be CLANG_ENABLE_STATIC_LIBRARIES=ON 
CLANG_ENABLE_SHARED_LIBRARIeS=OFF

Suggested by @beanz


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61804

Files:
  clang/CMakeLists.txt
  clang/cmake/modules/AddClang.cmake
  llvm/cmake/modules/AddLLVM.cmake


Index: llvm/cmake/modules/AddLLVM.cmake
===
--- llvm/cmake/modules/AddLLVM.cmake
+++ llvm/cmake/modules/AddLLVM.cmake
@@ -442,20 +442,20 @@
   endif()
 
   if(ARG_SHARED AND ARG_STATIC)
-# static
-set(name_static "${name}_static")
+# Do shared first
+set(name_shared "${name}_shared")
 if(ARG_OUTPUT_NAME)
   set(output_name OUTPUT_NAME "${ARG_OUTPUT_NAME}")
 endif()
 # DEPENDS has been appended to LLVM_COMMON_LIBS.
-llvm_add_library(${name_static} STATIC
+llvm_add_library(${name_shared} SHARED
   ${output_name}
   OBJLIBS ${ALL_FILES} # objlib
   LINK_LIBS ${ARG_LINK_LIBS}
   LINK_COMPONENTS ${ARG_LINK_COMPONENTS}
   )
-# FIXME: Add name_static to anywhere in TARGET ${name}'s PROPERTY.
-set(ARG_STATIC)
+# FIXME: Add name_shared to anywhere in TARGET ${name}'s PROPERTY.
+set(ARG_SHARED)
   endif()
 
   if(ARG_MODULE)
Index: clang/cmake/modules/AddClang.cmake
===
--- clang/cmake/modules/AddClang.cmake
+++ clang/cmake/modules/AddClang.cmake
@@ -44,8 +44,8 @@
 
 macro(add_clang_library name)
   cmake_parse_arguments(ARG
-"SHARED"
-""
+"SHARED;STATIC"
+"OUTPUT_NAME"
 "ADDITIONAL_HEADERS"
 ${ARGN})
   set(srcs)
@@ -80,10 +80,20 @@
   ${ARG_ADDITIONAL_HEADERS} # It may contain unparsed unknown args.
   )
   endif()
-  if(ARG_SHARED)
+  if(ARG_SHARED OR CLANG_ENABLE_SHARED_LIBRARIES)
 set(ARG_ENABLE_SHARED SHARED)
   endif()
-  llvm_add_library(${name} ${ARG_ENABLE_SHARED} ${ARG_UNPARSED_ARGUMENTS} 
${srcs})
+  if(ARG_STATIC OR CLANG_ENABLE_STATIC_LIBRARIES)
+set(ARG_ENABLE_STATIC STATIC)
+  endif()
+  if(ARG_OUTPUT_NAME)
+set(NAME_OUTPUT OUTPUT_NAME ${ARG_OUTPUT_NAME})
+  elseif(ARG_SHARED AND ARG_STATIC)
+set(NAME_OUTPUT OUTPUT_NAME ${name})
+  else()
+set(NAME_OUTPUT)
+  endif()
+  llvm_add_library(${name} ${NAME_OUTPUT} ${ARG_ENABLE_SHARED} 
${ARG_ENABLE_STATIC} ${ARG_UNPARSED_ARGUMENTS} ${srcs})
 
   if(TARGET ${name})
 target_link_libraries(${name} INTERFACE ${LLVM_COMMON_LIBS})
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -447,6 +447,9 @@
 option(CLANG_ENABLE_ARCMT "Build ARCMT." ON)
 option(CLANG_ENABLE_STATIC_ANALYZER "Build static analyzer." ON)
 
+option(CLANG_ENABLE_SHARED_LIBRARIES "Build libclang* as shared libraries." 
OFF)
+option(CLANG_ENABLE_STATIC_LIBRARIES "Build libclang* as static libraries." ON)
+
 option(CLANG_ENABLE_PROTO_FUZZER "Build Clang protobuf fuzzer." OFF)
 
 if(NOT CLANG_ENABLE_STATIC_ANALYZER AND CLANG_ENABLE_ARCMT)


Index: llvm/cmake/modules/AddLLVM.cmake
===
--- llvm/cmake/modules/AddLLVM.cmake
+++ llvm/cmake/modules/AddLLVM.cmake
@@ -442,20 +442,20 @@
   endif()
 
   if(ARG_SHARED AND ARG_STATIC)
-# static
-set(name_static "${name}_static")
+# Do shared first
+set(name_shared "${name}_shared")
 if(ARG_OUTPUT_NAME)
   set(output_name OUTPUT_NAME "${ARG_OUTPUT_NAME}")
 endif()
 # DEPENDS has been appended to LLVM_COMMON_LIBS.
-llvm_add_library(${name_static} STATIC
+llvm_add_library(${name_shared} SHARED
   ${output_name}
   OBJLIBS ${ALL_FILES} # objlib
   LINK_LIBS ${ARG_LINK_LIBS}
   LINK_COMPONENTS ${ARG_LINK_COMPONENTS}
   )
-# FIXME: Add name_static to anywhere in TARGET ${name}'s PROPERTY.
-set(ARG_STATIC)
+# FIXME: Add name_shared to anywhere in TARGET ${name}'s PROPERTY.
+set(ARG_SHARED)
   endif()
 
   if(ARG_MODULE)
Index: clang/cmake/modules/AddClang.cmake
===
--- clang/cmake/modules/AddClang.cmake
+++ clang/cmake/modules/AddClang.cmake
@@ -44,8 +44,8 @@
 
 macro(add_clang_library name)
   cmake_parse_arguments(ARG
-"SHARED"
-""
+"SHARED;STATIC"
+"OUTPUT_NAME"
 "ADDITIONAL_HEADERS"
 ${ARGN})
   set(srcs)
@@ -80,10 +80,20 @@
   ${ARG_ADDITIONAL_HEADERS} # It may contain unparsed unknown args.
   )
   endif()
-  if(ARG_SHARED)
+  if(ARG_SHARED OR CLANG_ENABLE_SHARED_LIBRARIES)
 set(ARG_ENABLE_SHARED SHARED)
   endif()
-  llvm_add_library(${name} ${ARG_ENABLE_SHARED} ${ARG_UNPARSED_ARGUMENTS} ${srcs})
+  if(ARG_STATIC OR CLANG_ENABLE_STATIC_LIBRARIES)
+set(ARG_ENABLE_STATIC STATIC)
+  endif()
+  if(ARG_OUTPUT_NAME)
+set(NAME_OUTPUT OUTPUT_NAME 

[PATCH] D61804: Support building shared and static libraries simultaneously

[Wink Saville via Phabricator via cfe-commits]

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

In D61804#1499267 , @beanz wrote:

> As an additional note, Arch linux should not be building clang with 
> `BUILD_SHARED_LIBS` nor should it be distributing those ,so files. That isn't 
> a supported configuration for Clang deployment.


Agreed, but I'd like to give them a choice of making both if I can get this 
accepted. It looks to me that llvm, libunwind and libcxx support building both, 
so the goal isn't unprecedented,




Comment at: clang/CMakeLists.txt:451
+option(CLANG_ENABLE_SHARED_LIBRARIES "Build libclang* as shared libraries." ON)
+option(CLANG_ENABLE_STATIC_LIBRARIES "Build libclang* as static libraries." ON)
+

beanz wrote:
> These shouldn't both default to `On`, that is a change in behavior that would 
> be a build-time regression for anyone not interested in building shared 
> libraries. `STATIC` should default `On`, and `SHARED` default `Off`.
> 
> Also you need to check that one of the two options is enabled. If both are 
> `Off` confusing things will happen.
I'll change it to default SHARED OFF and STATIC ON.

When both are off it acts as it does without these changes, the default will 
depend on BUILD_SHARED_LIBS:
```
# llvm_add_library(name sources...
#   SHARED;STATIC
# STATIC by default w/o BUILD_SHARED_LIBS.
# SHARED by default w/  BUILD_SHARED_LIBS.
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61804



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

[PATCH] D61804: Support building shared and static libraries simultaneously

[Wink Saville via Phabricator via cfe-commits]

winksaville added a comment.

In D61804#1499259 , @beanz wrote:

> I question the general utility of this patch, and I don't really think this 
> does anything we want the build system doing.


When you say you don't think the build system should do this, what do you mean?
The llvm_add_library routine without my change already supports generating both,
my change just changes the default cmake entities created from being:

  shared: ${name}
  static: ${name}_static

to:

  shared: ${name}_shared
  static: ${name}



> If you specify `STATIC` and `SHARED` to `llvm_add_library` it creates the 
> shared library target as the one that everything links against, which is 
> exactly what you don't want.

With the change I've made when both are specified the default cmake entity, 
${name}, is the static libraries. When neither are specified the default is 
static or shared depending on BUILD_SHARED_LIBS which is unchanged.

> There are two primary reasons why `BUILD_SHARED_LIBS` is not recommended for 
> non-development builds both have to do with how shared object linking works:
> 
> (1) Is that clang when linked against shared libraries is *way* slower (I 
> think last I measured was over 10%). This is caused by the need for resolving 
> linkage at process launch every time a process is launched.
> 
> (2) Is that LLVM relies on a strange mess of static initializers used for 
> static registration. The most problematic one is cl::opt. When you link a 
> static archive only the object files that contain unresolved symbols get 
> pulled in, when you link against a shared object, all of it gets pulled in. 
> Because static archives selectively pull in object files (and their static 
> initializers) we have had issues in the past where the static build of LLVM 
> works, but the shared build produces runtime errors from cl::opt.
> 
> I'm curious to understand the problem you are trying to solve. From other 
> posts on the list it really sounds to me like you want a variant of libClang 
> that exports the entire Clang API surface rather than just the C API. I think 
> there are better ways to accomplish that.

The problem is that some people want to use shared libraries and some want 
static. In particular Arch Linux prefers shared over static and with clang 
there is no option for building both so they end up building only shared. I'm 
trying to allow both to be built and ask the Arch Linux people to distribute 
both.

> Exposing Clang's libraries for piecemeal consumption is a much bigger problem 
> than just the build system not building libClang*.so. For example, Clang has 
> no equivalent of llvm-config, so there is no external representation of the 
> clang dependency graph that users can rely on.
> 
> This all aside, I also have issues with this implementation described inline 
> below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61804



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

[PATCH] D61804: Support building shared and static libraries simultaneously

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

As an additional note, Arch linux should not be building clang with 
`BUILD_SHARED_LIBS` nor should it be distributing those ,so files. That isn't a 
supported configuration for Clang deployment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61804



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

[PATCH] D61804: Support building shared and static libraries simultaneously

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

I question the general utility of this patch, and I don't really think this 
does anything we want the build system doing.

If you specify `STATIC` and `SHARED` to `llvm_add_library` it creates the 
shared library target as the one that everything links against, which is 
exactly what you don't want. There are two primary reasons why 
`BUILD_SHARED_LIBS` is not recommended for non-development builds both have to 
do with how shared object linking works:

(1) Is that clang when linked against shared libraries is *way* slower (I think 
last I measured was over 10%). This is caused by the need for resolving linkage 
at process launch every time a process is launched.

(2) Is that LLVM relies on a strange mess of static initializers used for 
static registration. The most problematic one is cl::opt. When you link a 
static archive only the object files that contain unresolved symbols get pulled 
in, when you link against a shared object, all of it gets pulled in. Because 
static archives selectively pull in object files (and their static 
initializers) we have had issues in the past where the static build of LLVM 
works, but the shared build produces runtime errors from cl::opt.

I'm curious to understand the problem you are trying to solve. From other posts 
on the list it really sounds to me like you want a variant of libClang that 
exports the entire Clang API surface rather than just the C API. I think there 
are better ways to accomplish that.

Exposing Clang's libraries for piecemeal consumption is a much bigger problem 
than just the build system not building libClang*.so. For example, Clang has no 
equivalent of llvm-config, so there is no external representation of the clang 
dependency graph that users can rely on.

This all aside, I also have issues with this implementation described inline 
below.




Comment at: clang/CMakeLists.txt:451
+option(CLANG_ENABLE_SHARED_LIBRARIES "Build libclang* as shared libraries." ON)
+option(CLANG_ENABLE_STATIC_LIBRARIES "Build libclang* as static libraries." ON)
+

These shouldn't both default to `On`, that is a change in behavior that would 
be a build-time regression for anyone not interested in building shared 
libraries. `STATIC` should default `On`, and `SHARED` default `Off`.

Also you need to check that one of the two options is enabled. If both are 
`Off` confusing things will happen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61804



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

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

[Tamás Zolnai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL360540: [clang-tidy] new check: 
bugprone-unhandled-self-assignment (authored by ztamas, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D60507?vs=198789=199166#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60507

Files:
  clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/trunk/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
  clang-tools-extra/trunk/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.h
  clang-tools-extra/trunk/docs/ReleaseNotes.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
  clang-tools-extra/trunk/test/clang-tidy/bugprone-unhandled-self-assignment.cpp

Index: clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
===
--- clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -46,6 +46,7 @@
 #include "TooSmallLoopVariableCheck.h"
 #include "UndefinedMemoryManipulationCheck.h"
 #include "UndelegatedConstructorCheck.h"
+#include "UnhandledSelfAssignmentCheck.h"
 #include "UnusedRaiiCheck.h"
 #include "UnusedReturnValueCheck.h"
 #include "UseAfterMoveCheck.h"
@@ -132,6 +133,8 @@
 "bugprone-undefined-memory-manipulation");
 CheckFactories.registerCheck(
 "bugprone-undelegated-constructor");
+CheckFactories.registerCheck(
+"bugprone-unhandled-self-assignment");
 CheckFactories.registerCheck(
 "bugprone-unused-raii");
 CheckFactories.registerCheck(
Index: clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
===
--- clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
+++ clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
@@ -38,6 +38,7 @@
   TooSmallLoopVariableCheck.cpp
   UndefinedMemoryManipulationCheck.cpp
   UndelegatedConstructorCheck.cpp
+  UnhandledSelfAssignmentCheck.cpp
   UnusedRaiiCheck.cpp
   UnusedReturnValueCheck.cpp
   UseAfterMoveCheck.cpp
Index: clang-tools-extra/trunk/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
@@ -0,0 +1,99 @@
+//===--- UnhandledSelfAssignmentCheck.cpp - clang-tidy ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "UnhandledSelfAssignmentCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+void UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+return;
+
+  // We don't care about deleted, default or implicit operator implementations.
+  const auto IsUserDefined = cxxMethodDecl(
+  isDefinition(), unless(anyOf(isDeleted(), isImplicit(), isDefaulted(;
+
+  // We don't need to worry when a copy assignment operator gets the other
+  // object by value.
+  const auto HasReferenceParam =
+  cxxMethodDecl(hasParameter(0, parmVarDecl(hasType(referenceType();
+
+  // Self-check: Code compares something with 'this' pointer. We don't check
+  // whether it is actually the parameter what we compare.
+  const auto HasNoSelfCheck = cxxMethodDecl(unless(hasDescendant(
+  binaryOperator(anyOf(hasOperatorName("=="), hasOperatorName("!=")),
+ has(ignoringParenCasts(cxxThisExpr()));
+
+  // Both copy-and-swap and copy-and-move method creates a copy first and
+  // assign it to 'this' with swap or move.
+  // In the non-template case, we can search for the copy constructor call.
+  const auto HasNonTemplateSelfCopy = cxxMethodDecl(
+  ofClass(cxxRecordDecl(unless(hasAncestor(classTemplateDecl(),
+  hasDescendant(cxxConstructExpr(hasDeclaration(cxxConstructorDecl(
+  isCopyConstructor(), ofClass(equalsBoundNode("class")));
+
+  // In the template case, we need to handle two separate cases: 1) a local
+  // variable is created with the copy, 2) copy is created only as a temporary
+  // object.
+  const auto HasTemplateSelfCopy = cxxMethodDecl(
+  

[clang-tools-extra] r360540 - [clang-tidy] new check: bugprone-unhandled-self-assignment

[Tamas Zolnai via cfe-commits]

Author: ztamas
Date: Sun May 12 05:23:56 2019
New Revision: 360540

URL: http://llvm.org/viewvc/llvm-project?rev=360540=rev
Log:
[clang-tidy] new check: bugprone-unhandled-self-assignment

Summary:
This check searches for copy assignment operators which might not handle 
self-assignment properly. There are three patterns of
handling a self assignment situation: self check, copy-and-swap or the less 
common copy-and-move. The new check warns if none of
these patterns is found in a user defined implementation.

See also:
OOP54-CPP. Gracefully handle self-copy assignment
https://wiki.sei.cmu.edu/confluence/display/cplusplus/OOP54-CPP.+Gracefully+handle+self-copy+assignment

Reviewers: JonasToth, alexfh, hokein, aaron.ballman

Subscribers: riccibruno, Eugene.Zelenko, mgorny, xazax.hun, cfe-commits

Tags: #clang, #clang-tools-extra

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

Added:
clang-tools-extra/trunk/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
clang-tools-extra/trunk/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.h

clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst

clang-tools-extra/trunk/test/clang-tidy/bugprone-unhandled-self-assignment.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp?rev=360540=360539=360540=diff
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp Sun May 
12 05:23:56 2019
@@ -46,6 +46,7 @@
 #include "TooSmallLoopVariableCheck.h"
 #include "UndefinedMemoryManipulationCheck.h"
 #include "UndelegatedConstructorCheck.h"
+#include "UnhandledSelfAssignmentCheck.h"
 #include "UnusedRaiiCheck.h"
 #include "UnusedReturnValueCheck.h"
 #include "UseAfterMoveCheck.h"
@@ -132,6 +133,8 @@ public:
 "bugprone-undefined-memory-manipulation");
 CheckFactories.registerCheck(
 "bugprone-undelegated-constructor");
+CheckFactories.registerCheck(
+"bugprone-unhandled-self-assignment");
 CheckFactories.registerCheck(
 "bugprone-unused-raii");
 CheckFactories.registerCheck(

Modified: clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt?rev=360540=360539=360540=diff
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt Sun May 12 
05:23:56 2019
@@ -38,6 +38,7 @@ add_clang_library(clangTidyBugproneModul
   TooSmallLoopVariableCheck.cpp
   UndefinedMemoryManipulationCheck.cpp
   UndelegatedConstructorCheck.cpp
+  UnhandledSelfAssignmentCheck.cpp
   UnusedRaiiCheck.cpp
   UnusedReturnValueCheck.cpp
   UseAfterMoveCheck.cpp

Added: 
clang-tools-extra/trunk/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp?rev=360540=auto
==
--- 
clang-tools-extra/trunk/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp 
(added)
+++ 
clang-tools-extra/trunk/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp 
Sun May 12 05:23:56 2019
@@ -0,0 +1,99 @@
+//===--- UnhandledSelfAssignmentCheck.cpp - clang-tidy 
===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "UnhandledSelfAssignmentCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+void UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+return;
+
+  // We don't care about deleted, default or implicit operator implementations.
+  const auto IsUserDefined = cxxMethodDecl(
+  isDefinition(), unless(anyOf(isDeleted(), isImplicit(), isDefaulted(;
+
+  // We don't need to worry when a copy assignment operator gets the other
+  // object by value.
+  const auto HasReferenceParam =
+  cxxMethodDecl(hasParameter(0, parmVarDecl(hasType(referenceType();
+

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

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rC360539: make -ftime-trace also print template arguments 
(authored by llunak, committed by ).

Repository:
  rC Clang

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

https://reviews.llvm.org/D61822

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


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


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

r360538 - Reject attempts to call non-static member functions on objects outside

[Richard Smith via cfe-commits]

Author: rsmith
Date: Sun May 12 02:39:08 2019
New Revision: 360538

URL: http://llvm.org/viewvc/llvm-project?rev=360538=rev
Log:
Reject attempts to call non-static member functions on objects outside
their lifetime in constant expressions.

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

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

This reinstates r360499, reverted in r360531.

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

Modified: cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td?rev=360538=360537=360538=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td Sun May 12 02:39:08 2019
@@ -67,13 +67,13 @@ def note_constexpr_past_end : Note<
   "%select{temporary|%2}1 is not a constant expression">;
 def note_constexpr_past_end_subobject : Note<
   "cannot %select{access base class of|access derived class of|access field 
of|"
-  "access array element of|ERROR|call member function on|"
+  "access array element of|ERROR|"
   "access real component of|access imaginary component of}0 "
   "pointer past the end of object">;
 def note_constexpr_null_subobject : Note<
   "cannot %select{access base class of|access derived class of|access field 
of|"
   "access array element of|perform pointer arithmetic on|"
-  "call member function on|access real component of|"
+  "access real component of|"
   "access imaginary component of}0 null pointer">;
 def note_constexpr_var_init_non_constant : Note<
   "initializer of %0 is not a constant expression">;
@@ -96,10 +96,10 @@ def note_constexpr_this : Note<
   "%select{|implicit }0use of 'this' pointer is only allowed within the "
   "evaluation of a call to a 'constexpr' member function">;
 def note_constexpr_lifetime_ended : Note<
-  "%select{read of|assignment to|increment of|decrement of}0 "
+  "%select{read of|assignment to|increment of|decrement of|member call on}0 "
   "%select{temporary|variable}1 whose lifetime has ended">;
 def note_constexpr_access_uninit : Note<
-  "%select{read of|assignment to|increment of|decrement of}0 "
+  "%select{read of|assignment to|increment of|decrement of|member call on}0 "
   "object outside its lifetime is not allowed in a constant expression">;
 def note_constexpr_use_uninit_reference : Note<
   "use of reference outside its lifetime "
@@ -108,10 +108,10 @@ def note_constexpr_modify_const_type : N
   "modification of object of const-qualified type %0 is not allowed "
   "in a constant expression">;
 def note_constexpr_access_volatile_type : Note<
-  "%select{read of|assignment to|increment of|decrement of}0 "
+  "%select{read of|assignment to|increment of|decrement of|}0 "
   "volatile-qualified type %1 is not allowed in a constant expression">;
 def note_constexpr_access_volatile_obj : Note<
-  "%select{read of|assignment to|increment of|decrement of}0 volatile "
+  "%select{read of|assignment to|increment of|decrement of|}0 volatile "
   "%select{temporary|object %2|member %2}1 is not allowed in "
   "a constant expression">;
 def note_constexpr_volatile_here : Note<
@@ -125,21 +125,21 @@ def note_constexpr_ltor_non_constexpr :
 def note_constexpr_ltor_incomplete_type : Note<
   "read of incomplete type %0 is not allowed in a constant expression">;
 def note_constexpr_access_null : Note<
-  "%select{read of|assignment to|increment of|decrement of}0 "
+  "%select{read of|assignment to|increment of|decrement of|member call on}0 "
   "dereferenced null pointer is not allowed in a constant expression">;
 def note_constexpr_access_past_end : Note<
-  "%select{read of|assignment to|increment of|decrement of}0 "
+  "%select{read of|assignment to|increment of|decrement of|member call on}0 "
   "dereferenced one-past-the-end pointer is not allowed in a constant 
expression">;
 def note_constexpr_access_unsized_array : Note<
-  "%select{read of|assignment to|increment of|decrement of}0 "
-  "pointer to element of array without known bound "
+  "%select{read of|assignment to|increment of|decrement of|member call on}0 "
+  "element of array without known bound "
   "is not allowed in a constant expression">;
 def note_constexpr_access_inactive_union_member : Note<
-  "%select{read of|assignment to|increment of|decrement of}0 "
+  "%select{read of|assignment to|increment of|decrement of|member call on}0 "
   "member %1 of union with %select{active member %3|no active member}2 "
   "is not allowed in a constant 

r360537 - Fix handling of objects under construction during constant expression

[Richard Smith via cfe-commits]

Author: rsmith
Date: Sun May 12 01:57:59 2019
New Revision: 360537

URL: http://llvm.org/viewvc/llvm-project?rev=360537=rev
Log:
Fix handling of objects under construction during constant expression
evaluation.

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

This reinstates r360464 (reverted in r360531) with a workaround for an
MSVC bug that previously caused the Windows bots to fail.

Modified:
cfe/trunk/include/clang/AST/APValue.h
cfe/trunk/include/clang/AST/Redeclarable.h
cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
cfe/trunk/lib/AST/APValue.cpp
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/test/SemaCXX/constant-expression-cxx1y.cpp

Modified: cfe/trunk/include/clang/AST/APValue.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/APValue.h?rev=360537=360536=360537=diff
==
--- cfe/trunk/include/clang/AST/APValue.h (original)
+++ cfe/trunk/include/clang/AST/APValue.h Sun May 12 01:57:59 2019
@@ -96,10 +96,14 @@ public:
   return Version;
 }
 
-bool operator==(const LValueBase ) const {
-  return Ptr == Other.Ptr && CallIndex == Other.CallIndex &&
- Version == Other.Version;
+friend bool operator==(const LValueBase , const LValueBase ) {
+  return LHS.Ptr == RHS.Ptr && LHS.CallIndex == RHS.CallIndex &&
+ LHS.Version == RHS.Version;
 }
+friend bool operator!=(const LValueBase , const LValueBase ) {
+  return !(LHS == RHS);
+}
+friend llvm::hash_code hash_value(const LValueBase );
 
   private:
 PtrTy Ptr;

Modified: cfe/trunk/include/clang/AST/Redeclarable.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Redeclarable.h?rev=360537=360536=360537=diff
==
--- cfe/trunk/include/clang/AST/Redeclarable.h (original)
+++ cfe/trunk/include/clang/AST/Redeclarable.h Sun May 12 01:57:59 2019
@@ -361,6 +361,13 @@ public:
   decl_type *() { return *Ptr; }
   const decl_type *() const { return *Ptr; }
 
+  friend bool operator==(CanonicalDeclPtr LHS, CanonicalDeclPtr RHS) {
+return LHS.Ptr == RHS.Ptr;
+  }
+  friend bool operator!=(CanonicalDeclPtr LHS, CanonicalDeclPtr RHS) {
+return LHS.Ptr != RHS.Ptr;
+  }
+
 private:
   friend struct llvm::DenseMapInfo>;
 

Modified: cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td?rev=360537=360536=360537=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td Sun May 12 01:57:59 2019
@@ -114,6 +114,8 @@ def note_constexpr_access_volatile_obj :
   "%select{read of|assignment to|increment of|decrement of}0 volatile "
   "%select{temporary|object %2|member %2}1 is not allowed in "
   "a constant expression">;
+def note_constexpr_volatile_here : Note<
+  "volatile %select{temporary created|object declared|member declared}0 here">;
 def note_constexpr_ltor_mutable : Note<
   "read of mutable member %0 is not allowed in a constant expression">;
 def note_constexpr_ltor_non_const_int : Note<

Modified: cfe/trunk/lib/AST/APValue.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/APValue.cpp?rev=360537=360536=360537=diff
==
--- cfe/trunk/lib/AST/APValue.cpp (original)
+++ cfe/trunk/lib/AST/APValue.cpp Sun May 12 01:57:59 2019
@@ -58,13 +58,16 @@ llvm::DenseMapInfo::getTombstoneKey());
 }
 
+namespace clang {
+llvm::hash_code hash_value(const APValue::LValueBase ) {
+  return llvm::hash_combine(Base.getOpaqueValue(), Base.getCallIndex(),
+Base.getVersion());
+}
+}
+
 unsigned llvm::DenseMapInfo::getHashValue(
 const clang::APValue::LValueBase ) {
-  llvm::FoldingSetNodeID ID;
-  ID.AddPointer(Base.getOpaqueValue());
-  ID.AddInteger(Base.getCallIndex());
-  ID.AddInteger(Base.getVersion());
-  return ID.ComputeHash();
+  return hash_value(Base);
 }
 
 bool llvm::DenseMapInfo::isEqual(

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=360537=360536=360537=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Sun May 12 01:57:59 2019
@@ -622,6 +622,40 @@ namespace {
 }
   };
 
+  /// A reference to an object whose construction we are currently evaluating.
+  struct ObjectUnderConstruction {
+APValue::LValueBase Base;
+ArrayRef Path;
+friend bool operator==(const ObjectUnderConstruction ,
+   const