[PATCH] D54438: [analyzer][WIP] Reimplement dependencies between checkers

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

Szelethus added a comment.

Okay. I was wrong, and you were right.

`MallocChecker` seriously suffers from overcrowding. Especially with the 
addition of `InnerPointerChecker`, it should be split up, but doing that is 
very much avoidable for the purposes of this effort while still achieving every 
goal of it (which I'll detail in the next couple points). But, I spent so much 
time with it, it kinda grew on me, so I'll probably tackle the problem on my 
downtime.

The change of course as follows, checkmarks indicate that I already have a 
working solution locally: (largely copied from D54438#1296695 
)

- ✔️ Introduce the boolean `ento::shouldRegister##CHECKERNAME(const LangOptions 
)` function very similarly to `ento::register##CHECKERNAME`. This will force 
every checker to implement this function, but maybe it isn't that bad: I saw a 
lot of ObjC or C++ specific checkers that should probably not register 
themselves based on some `LangOptions` (mine too), but they do anyways. Add an 
assert when `getChecker` is called on a checker that isn't already registered.
  - ❌ What I didn't do just yet but might be a good idea, is to rename 
`register##CHECKERNAME` to `enable##CHECKERNAME`, and let's reserve the term 
"registering" to `CheckerRegistry`.
  - This patch will //not// feature any functional change, despite the 
observation I just made.

- ✔️ There are in fact a variety of checkers that contain subcheckers like 
`MallocChecker`, which I think is all good, but the concept that if a 
subchecker is enabled it only sort of registeres the main checker (in 
`MallocChecker`'s case it enables modeling, but not reporting) is very hard to 
follow. I propose that all such checkers should clearly have a base, called 
something `DynamicMemoryModeling`, or `IteratorModeling` etc.

- ✔️ `UnixAPI` contains a dual checker, but they actually don't interact at 
all, split them up.

- ✖️ Introduce dependencies, all checkers register themselves and only 
themselves. On my local branch I also like that it's super obvious which 
checker is interacting with which one, for example, `IteratorChecker` also has 
multiple parts, but with this patch, it's super obvious from a glance at 
`Checkers.td`.
  - ✔️ We can actually ensure that dependencies are registered before the 
checkers that depend on them, thanks to asserts added in the first part.
  - ❌ Add an assert when `CheckerManager::registerChecker` is called on an 
already registered checker.

- ❌ Move `CheckerManager::registerChecker` out of the registry functions.
  - ❌ Since `shouldRegister##CHECKERNAME` is a thing, we can move everything to 
the checker's constructor, supply a `CheckerManager`, eliminating the function 
entirely.
  - ❌ At long last, get rid of `CheckerManager::setCurrentCheckerName` and 
`CheckerManager::getCurrentCheckerName`.
- ❌ It was discussed multiple times (D48285#inline-423172 
, D49438#inline-433993 
), that acquiring the options 
for the checker isn't too easy, as it's name will be assigned only later on, so 
currently all checkers initialize their options past construction. This can be 
solved either by supplying the checker's name to every constructor, or simply 
storing all enabled checkers in `AnalyzerOptions`, and acquire it from there. 
I'll see.

- ✖️ Properly document why `CheckerRegistry` and `CheckerManager` are separate 
entities, how are the tblgen files processed, how are dependencies handled, and 
so on.

- ✖️ Rebase my local checker option-related branches, and celebrate. I wouldn't 
like to go in any more detail, who knows what lies ahead.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54438



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

[PATCH] D55121: Make several Python scripts portable across Python2 and Python 3

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Can you split off the pure modernisation changes like new exception or print ? 
Those are completely non-contentious changes after all. I generally do not like 
the range and list related changes as many instances are clear regressions for 
the 2.x case. filter to list comprehension should IMO be a separate change as 
well, but those are much less problematic and often an improvement in terms of 
both performance and readability.


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

https://reviews.llvm.org/D55121



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

[PATCH] D55139: [clangd] Avoid memory-mapping files on Windows

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle added a comment.

Hi Ilya. Does this apply to compile_commands.json too? I've seen that problem 
for that file as well. If not, I understand it can be another patch, just 
curious.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55139



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

[PATCH] D55069: Extend the CommentVisitor with parameter types

[Stephen Kelly via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL348100: Extend the CommentVisitor with parameter types 
(authored by steveire, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55069

Files:
  cfe/trunk/include/clang/AST/CommentVisitor.h


Index: cfe/trunk/include/clang/AST/CommentVisitor.h
===
--- cfe/trunk/include/clang/AST/CommentVisitor.h
+++ cfe/trunk/include/clang/AST/CommentVisitor.h
@@ -19,14 +19,16 @@
 template  struct make_ptr { using type = T *; };
 template  struct make_const_ptr { using type = const T *; };
 
-template class Ptr, typename ImplClass, typename 
RetTy=void>
+template  class Ptr, typename ImplClass,
+  typename RetTy = void, class... ParamTys>
 class CommentVisitorBase {
 public:
 #define PTR(CLASS) typename Ptr::type
-#define DISPATCH(NAME, CLASS) \
- return static_cast(this)->visit ## 
NAME(static_cast(C))
+#define DISPATCH(NAME, CLASS)  
\
+  return static_cast(this)->visit##NAME(  
\
+  static_cast(C), std::forward(P)...)
 
-  RetTy visit(PTR(Comment) C) {
+  RetTy visit(PTR(Comment) C, ParamTys... P) {
 if (!C)
   return RetTy();
 
@@ -44,25 +46,26 @@
   // If the derived class does not implement a certain Visit* method, fall back
   // on Visit* method for the superclass.
 #define ABSTRACT_COMMENT(COMMENT) COMMENT
-#define COMMENT(CLASS, PARENT) \
-  RetTy visit ## CLASS(PTR(CLASS) C) { DISPATCH(PARENT, PARENT); }
+#define COMMENT(CLASS, PARENT) 
\
+  RetTy visit##CLASS(PTR(CLASS) C, ParamTys... P) { DISPATCH(PARENT, PARENT); }
 #include "clang/AST/CommentNodes.inc"
 #undef ABSTRACT_COMMENT
 #undef COMMENT
 
-  RetTy visitComment(PTR(Comment) C) { return RetTy(); }
+  RetTy visitComment(PTR(Comment) C, ParamTys... P) { return RetTy(); }
 
 #undef PTR
 #undef DISPATCH
 };
 
-template
-class CommentVisitor :
-public CommentVisitorBase {};
-
-template
-class ConstCommentVisitor :
-public CommentVisitorBase {};
+template 
+class CommentVisitor
+: public CommentVisitorBase {};
+
+template 
+class ConstCommentVisitor
+: public CommentVisitorBase 
{
+};
 
 } // namespace comments
 } // namespace clang


Index: cfe/trunk/include/clang/AST/CommentVisitor.h
===
--- cfe/trunk/include/clang/AST/CommentVisitor.h
+++ cfe/trunk/include/clang/AST/CommentVisitor.h
@@ -19,14 +19,16 @@
 template  struct make_ptr { using type = T *; };
 template  struct make_const_ptr { using type = const T *; };
 
-template class Ptr, typename ImplClass, typename RetTy=void>
+template  class Ptr, typename ImplClass,
+  typename RetTy = void, class... ParamTys>
 class CommentVisitorBase {
 public:
 #define PTR(CLASS) typename Ptr::type
-#define DISPATCH(NAME, CLASS) \
- return static_cast(this)->visit ## NAME(static_cast(C))
+#define DISPATCH(NAME, CLASS)  \
+  return static_cast(this)->visit##NAME(  \
+  static_cast(C), std::forward(P)...)
 
-  RetTy visit(PTR(Comment) C) {
+  RetTy visit(PTR(Comment) C, ParamTys... P) {
 if (!C)
   return RetTy();
 
@@ -44,25 +46,26 @@
   // If the derived class does not implement a certain Visit* method, fall back
   // on Visit* method for the superclass.
 #define ABSTRACT_COMMENT(COMMENT) COMMENT
-#define COMMENT(CLASS, PARENT) \
-  RetTy visit ## CLASS(PTR(CLASS) C) { DISPATCH(PARENT, PARENT); }
+#define COMMENT(CLASS, PARENT) \
+  RetTy visit##CLASS(PTR(CLASS) C, ParamTys... P) { DISPATCH(PARENT, PARENT); }
 #include "clang/AST/CommentNodes.inc"
 #undef ABSTRACT_COMMENT
 #undef COMMENT
 
-  RetTy visitComment(PTR(Comment) C) { return RetTy(); }
+  RetTy visitComment(PTR(Comment) C, ParamTys... P) { return RetTy(); }
 
 #undef PTR
 #undef DISPATCH
 };
 
-template
-class CommentVisitor :
-public CommentVisitorBase {};
-
-template
-class ConstCommentVisitor :
-public CommentVisitorBase {};
+template 
+class CommentVisitor
+: public CommentVisitorBase {};
+
+template 
+class ConstCommentVisitor
+: public CommentVisitorBase {
+};
 
 } // namespace comments
 } // namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55070: Replace FullComment member being visited with parameter

[Stephen Kelly via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL348101: Replace FullComment member being visited with 
parameter (authored by steveire, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55070

Files:
  cfe/trunk/lib/AST/ASTDumper.cpp

Index: cfe/trunk/lib/AST/ASTDumper.cpp
===
--- cfe/trunk/lib/AST/ASTDumper.cpp
+++ cfe/trunk/lib/AST/ASTDumper.cpp
@@ -109,8 +109,11 @@
   };
 
   class ASTDumper
-  : public ConstDeclVisitor, public ConstStmtVisitor,
-public ConstCommentVisitor, public TypeVisitor {
+  : public ConstDeclVisitor,
+public ConstStmtVisitor,
+public ConstCommentVisitor,
+public TypeVisitor {
+
 raw_ostream 
 const CommandTraits *Traits;
 const SourceManager *SM;
@@ -139,9 +142,6 @@
 const char *LastLocFilename = "";
 unsigned LastLocLine = ~0U;
 
-/// The \c FullComment parent of the comment being dumped.
-const FullComment *FC = nullptr;
-
 const bool ShowColors;
 
 /// Dump a child of the current node.
@@ -161,8 +161,7 @@
 return;
   }
 
-  const FullComment *OrigFC = FC;
-  auto dumpWithIndent = [this, doDumpChild, OrigFC](bool isLastChild) {
+  auto dumpWithIndent = [this, doDumpChild](bool isLastChild) {
 // Print out the appropriate tree structure and work out the prefix for
 // children of this node. For instance:
 //
@@ -186,7 +185,6 @@
 FirstChild = true;
 unsigned Depth = Pending.size();
 
-FC = OrigFC;
 doDumpChild();
 
 // If any children are left, they're the last at their nesting level.
@@ -584,21 +582,30 @@
 
 // Comments.
 const char *getCommandName(unsigned CommandID);
-void dumpComment(const Comment *C);
+void dumpComment(const Comment *C, const FullComment *FC);
 
 // Inline comments.
-void visitTextComment(const TextComment *C);
-void visitInlineCommandComment(const InlineCommandComment *C);
-void visitHTMLStartTagComment(const HTMLStartTagComment *C);
-void visitHTMLEndTagComment(const HTMLEndTagComment *C);
+void visitTextComment(const TextComment *C, const FullComment *FC);
+void visitInlineCommandComment(const InlineCommandComment *C,
+   const FullComment *FC);
+void visitHTMLStartTagComment(const HTMLStartTagComment *C,
+  const FullComment *FC);
+void visitHTMLEndTagComment(const HTMLEndTagComment *C,
+const FullComment *FC);
 
 // Block comments.
-void visitBlockCommandComment(const BlockCommandComment *C);
-void visitParamCommandComment(const ParamCommandComment *C);
-void visitTParamCommandComment(const TParamCommandComment *C);
-void visitVerbatimBlockComment(const VerbatimBlockComment *C);
-void visitVerbatimBlockLineComment(const VerbatimBlockLineComment *C);
-void visitVerbatimLineComment(const VerbatimLineComment *C);
+void visitBlockCommandComment(const BlockCommandComment *C,
+  const FullComment *FC);
+void visitParamCommandComment(const ParamCommandComment *C,
+  const FullComment *FC);
+void visitTParamCommandComment(const TParamCommandComment *C,
+   const FullComment *FC);
+void visitVerbatimBlockComment(const VerbatimBlockComment *C,
+   const FullComment *FC);
+void visitVerbatimBlockLineComment(const VerbatimBlockLineComment *C,
+   const FullComment *FC);
+void visitVerbatimLineComment(const VerbatimLineComment *C,
+  const FullComment *FC);
   };
 }
 
@@ -2640,13 +2647,10 @@
 void ASTDumper::dumpFullComment(const FullComment *C) {
   if (!C)
 return;
-
-  FC = C;
-  dumpComment(C);
-  FC = nullptr;
+  dumpComment(C, C);
 }
 
-void ASTDumper::dumpComment(const Comment *C) {
+void ASTDumper::dumpComment(const Comment *C, const FullComment *FC) {
   dumpChild([=] {
 if (!C) {
   ColorScope Color(OS, ShowColors, NullColor);
@@ -2660,18 +2664,19 @@
 }
 dumpPointer(C);
 dumpSourceRange(C->getSourceRange());
-ConstCommentVisitor::visit(C);
+ConstCommentVisitor::visit(C, FC);
 for (Comment::child_iterator I = C->child_begin(), E = C->child_end();
  I != E; ++I)
-  dumpComment(*I);
+  dumpComment(*I, FC);
   });
 }
 
-void ASTDumper::visitTextComment(const TextComment *C) {
+void ASTDumper::visitTextComment(const TextComment *C, const FullComment *) {
   OS << " Text=\"" << C->getText() << "\"";
 }
 
-void ASTDumper::visitInlineCommandComment(const InlineCommandComment *C) {
+void ASTDumper::visitInlineCommandComment(const 

r348100 - Extend the CommentVisitor with parameter types

[Stephen Kelly via cfe-commits]

Author: steveire
Date: Sun Dec  2 09:30:34 2018
New Revision: 348100

URL: http://llvm.org/viewvc/llvm-project?rev=348100=rev
Log:
Extend the CommentVisitor with parameter types

Summary:
This has precedent in the StmtVisitor.  This change will make it
possible to clean up the comment handling in ASTDumper.

Reviewers: aaron.ballman

Subscribers: cfe-commits

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

Modified:
cfe/trunk/include/clang/AST/CommentVisitor.h

Modified: cfe/trunk/include/clang/AST/CommentVisitor.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/CommentVisitor.h?rev=348100=348099=348100=diff
==
--- cfe/trunk/include/clang/AST/CommentVisitor.h (original)
+++ cfe/trunk/include/clang/AST/CommentVisitor.h Sun Dec  2 09:30:34 2018
@@ -19,14 +19,16 @@ namespace comments {
 template  struct make_ptr { using type = T *; };
 template  struct make_const_ptr { using type = const T *; };
 
-template class Ptr, typename ImplClass, typename 
RetTy=void>
+template  class Ptr, typename ImplClass,
+  typename RetTy = void, class... ParamTys>
 class CommentVisitorBase {
 public:
 #define PTR(CLASS) typename Ptr::type
-#define DISPATCH(NAME, CLASS) \
- return static_cast(this)->visit ## 
NAME(static_cast(C))
+#define DISPATCH(NAME, CLASS)  
\
+  return static_cast(this)->visit##NAME(  
\
+  static_cast(C), std::forward(P)...)
 
-  RetTy visit(PTR(Comment) C) {
+  RetTy visit(PTR(Comment) C, ParamTys... P) {
 if (!C)
   return RetTy();
 
@@ -44,25 +46,26 @@ public:
   // If the derived class does not implement a certain Visit* method, fall back
   // on Visit* method for the superclass.
 #define ABSTRACT_COMMENT(COMMENT) COMMENT
-#define COMMENT(CLASS, PARENT) \
-  RetTy visit ## CLASS(PTR(CLASS) C) { DISPATCH(PARENT, PARENT); }
+#define COMMENT(CLASS, PARENT) 
\
+  RetTy visit##CLASS(PTR(CLASS) C, ParamTys... P) { DISPATCH(PARENT, PARENT); }
 #include "clang/AST/CommentNodes.inc"
 #undef ABSTRACT_COMMENT
 #undef COMMENT
 
-  RetTy visitComment(PTR(Comment) C) { return RetTy(); }
+  RetTy visitComment(PTR(Comment) C, ParamTys... P) { return RetTy(); }
 
 #undef PTR
 #undef DISPATCH
 };
 
-template
-class CommentVisitor :
-public CommentVisitorBase {};
-
-template
-class ConstCommentVisitor :
-public CommentVisitorBase {};
+template 
+class CommentVisitor
+: public CommentVisitorBase {};
+
+template 
+class ConstCommentVisitor
+: public CommentVisitorBase 
{
+};
 
 } // namespace comments
 } // namespace clang


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

r348101 - Replace FullComment member being visited with parameter

[Stephen Kelly via cfe-commits]

Author: steveire
Date: Sun Dec  2 09:30:40 2018
New Revision: 348101

URL: http://llvm.org/viewvc/llvm-project?rev=348101=rev
Log:
Replace FullComment member being visited with parameter

Reviewers: aaron.ballman

Subscribers: cfe-commits

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

Modified:
cfe/trunk/lib/AST/ASTDumper.cpp

Modified: cfe/trunk/lib/AST/ASTDumper.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTDumper.cpp?rev=348101=348100=348101=diff
==
--- cfe/trunk/lib/AST/ASTDumper.cpp (original)
+++ cfe/trunk/lib/AST/ASTDumper.cpp Sun Dec  2 09:30:40 2018
@@ -109,8 +109,11 @@ namespace  {
   };
 
   class ASTDumper
-  : public ConstDeclVisitor, public ConstStmtVisitor,
-public ConstCommentVisitor, public TypeVisitor {
+  : public ConstDeclVisitor,
+public ConstStmtVisitor,
+public ConstCommentVisitor,
+public TypeVisitor {
+
 raw_ostream 
 const CommandTraits *Traits;
 const SourceManager *SM;
@@ -139,9 +142,6 @@ namespace  {
 const char *LastLocFilename = "";
 unsigned LastLocLine = ~0U;
 
-/// The \c FullComment parent of the comment being dumped.
-const FullComment *FC = nullptr;
-
 const bool ShowColors;
 
 /// Dump a child of the current node.
@@ -161,8 +161,7 @@ namespace  {
 return;
   }
 
-  const FullComment *OrigFC = FC;
-  auto dumpWithIndent = [this, doDumpChild, OrigFC](bool isLastChild) {
+  auto dumpWithIndent = [this, doDumpChild](bool isLastChild) {
 // Print out the appropriate tree structure and work out the prefix for
 // children of this node. For instance:
 //
@@ -186,7 +185,6 @@ namespace  {
 FirstChild = true;
 unsigned Depth = Pending.size();
 
-FC = OrigFC;
 doDumpChild();
 
 // If any children are left, they're the last at their nesting level.
@@ -584,21 +582,30 @@ namespace  {
 
 // Comments.
 const char *getCommandName(unsigned CommandID);
-void dumpComment(const Comment *C);
+void dumpComment(const Comment *C, const FullComment *FC);
 
 // Inline comments.
-void visitTextComment(const TextComment *C);
-void visitInlineCommandComment(const InlineCommandComment *C);
-void visitHTMLStartTagComment(const HTMLStartTagComment *C);
-void visitHTMLEndTagComment(const HTMLEndTagComment *C);
+void visitTextComment(const TextComment *C, const FullComment *FC);
+void visitInlineCommandComment(const InlineCommandComment *C,
+   const FullComment *FC);
+void visitHTMLStartTagComment(const HTMLStartTagComment *C,
+  const FullComment *FC);
+void visitHTMLEndTagComment(const HTMLEndTagComment *C,
+const FullComment *FC);
 
 // Block comments.
-void visitBlockCommandComment(const BlockCommandComment *C);
-void visitParamCommandComment(const ParamCommandComment *C);
-void visitTParamCommandComment(const TParamCommandComment *C);
-void visitVerbatimBlockComment(const VerbatimBlockComment *C);
-void visitVerbatimBlockLineComment(const VerbatimBlockLineComment *C);
-void visitVerbatimLineComment(const VerbatimLineComment *C);
+void visitBlockCommandComment(const BlockCommandComment *C,
+  const FullComment *FC);
+void visitParamCommandComment(const ParamCommandComment *C,
+  const FullComment *FC);
+void visitTParamCommandComment(const TParamCommandComment *C,
+   const FullComment *FC);
+void visitVerbatimBlockComment(const VerbatimBlockComment *C,
+   const FullComment *FC);
+void visitVerbatimBlockLineComment(const VerbatimBlockLineComment *C,
+   const FullComment *FC);
+void visitVerbatimLineComment(const VerbatimLineComment *C,
+  const FullComment *FC);
   };
 }
 
@@ -2640,13 +2647,10 @@ const char *ASTDumper::getCommandName(un
 void ASTDumper::dumpFullComment(const FullComment *C) {
   if (!C)
 return;
-
-  FC = C;
-  dumpComment(C);
-  FC = nullptr;
+  dumpComment(C, C);
 }
 
-void ASTDumper::dumpComment(const Comment *C) {
+void ASTDumper::dumpComment(const Comment *C, const FullComment *FC) {
   dumpChild([=] {
 if (!C) {
   ColorScope Color(OS, ShowColors, NullColor);
@@ -2660,18 +2664,19 @@ void ASTDumper::dumpComment(const Commen
 }
 dumpPointer(C);
 dumpSourceRange(C->getSourceRange());
-ConstCommentVisitor::visit(C);
+ConstCommentVisitor::visit(C, FC);
 for (Comment::child_iterator I = C->child_begin(), E = C->child_end();
  I != E; ++I)
-  dumpComment(*I);
+  dumpComment(*I, FC);
   });
 }
 
-void ASTDumper::visitTextComment(const TextComment *C) {
+void 

r348099 - Remove unecessary methods

[Stephen Kelly via cfe-commits]

Author: steveire
Date: Sun Dec  2 09:30:26 2018
New Revision: 348099

URL: http://llvm.org/viewvc/llvm-project?rev=348099=rev
Log:
Remove unecessary methods

The base class calls VisitExpr

Modified:
cfe/trunk/lib/AST/ASTDumper.cpp

Modified: cfe/trunk/lib/AST/ASTDumper.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTDumper.cpp?rev=348099=348098=348099=diff
==
--- cfe/trunk/lib/AST/ASTDumper.cpp (original)
+++ cfe/trunk/lib/AST/ASTDumper.cpp Sun Dec  2 09:30:26 2018
@@ -537,8 +537,6 @@ namespace  {
 void VisitFloatingLiteral(const FloatingLiteral *Node);
 void VisitStringLiteral(const StringLiteral *Str);
 void VisitInitListExpr(const InitListExpr *ILE);
-void VisitArrayInitLoopExpr(const ArrayInitLoopExpr *ILE);
-void VisitArrayInitIndexExpr(const ArrayInitIndexExpr *ILE);
 void VisitUnaryOperator(const UnaryOperator *Node);
 void VisitUnaryExprOrTypeTraitExpr(const UnaryExprOrTypeTraitExpr *Node);
 void VisitMemberExpr(const MemberExpr *Node);
@@ -2285,14 +2283,6 @@ void ASTDumper::VisitInitListExpr(const
   }
 }
 
-void ASTDumper::VisitArrayInitLoopExpr(const ArrayInitLoopExpr *E) {
-  VisitExpr(E);
-}
-
-void ASTDumper::VisitArrayInitIndexExpr(const ArrayInitIndexExpr *E) {
-  VisitExpr(E);
-}
-
 void ASTDumper::VisitUnaryOperator(const UnaryOperator *Node) {
   VisitExpr(Node);
   OS << " " << (Node->isPostfix() ? "postfix" : "prefix")


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

r348094 - Fix whitespace

[Stephen Kelly via cfe-commits]

Author: steveire
Date: Sun Dec  2 08:42:34 2018
New Revision: 348094

URL: http://llvm.org/viewvc/llvm-project?rev=348094=rev
Log:
Fix whitespace

Modified:
cfe/trunk/test/AST/ast-dump-array.cpp

Modified: cfe/trunk/test/AST/ast-dump-array.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/AST/ast-dump-array.cpp?rev=348094=348093=348094=diff
==
--- cfe/trunk/test/AST/ast-dump-array.cpp (original)
+++ cfe/trunk/test/AST/ast-dump-array.cpp Sun Dec  2 08:42:34 2018
@@ -6,5 +6,5 @@ void testArrayInitExpr()
 auto l = [a]{
 };
 // CHECK: |-ArrayInitLoopExpr 0x{{[^ ]*}}  'int [10]'
-// CHECK: | `-ArrayInitIndexExpr 0x{{[^ ]*}} <> 'unsigned 
long'
+// CHECK: | `-ArrayInitIndexExpr 0x{{[^ ]*}} <> 
'unsigned long'
 }


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

r348093 - Add dump tests for ArrayInitLoopExpr and ArrayInitIndexExpr

[Stephen Kelly via cfe-commits]

Author: steveire
Date: Sun Dec  2 08:36:23 2018
New Revision: 348093

URL: http://llvm.org/viewvc/llvm-project?rev=348093=rev
Log:
Add dump tests for ArrayInitLoopExpr and ArrayInitIndexExpr

Added:
cfe/trunk/test/AST/ast-dump-array.cpp

Added: cfe/trunk/test/AST/ast-dump-array.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/AST/ast-dump-array.cpp?rev=348093=auto
==
--- cfe/trunk/test/AST/ast-dump-array.cpp (added)
+++ cfe/trunk/test/AST/ast-dump-array.cpp Sun Dec  2 08:36:23 2018
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ast-dump %s | FileCheck 
-strict-whitespace %s
+
+void testArrayInitExpr()
+{
+int a[10];
+auto l = [a]{
+};
+// CHECK: |-ArrayInitLoopExpr 0x{{[^ ]*}}  'int [10]'
+// CHECK: | `-ArrayInitIndexExpr 0x{{[^ ]*}} <> 'unsigned 
long'
+}


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

[PATCH] D55068: NFC: Simplify dumpStmt child handling

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

Aaron, you added tests for the existing behavior which pass after this patch. 
Is there anything holding it up?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55068



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

[PATCH] D50852: [clang-tidy] abseil-auto-make-unique

[Roman Lebedev via Phabricator via cfe-commits]

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

Marking both of these as "changes requested" to highlight the similarity of 
issues in them.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D50852



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

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

[Roman Lebedev via Phabricator via cfe-commits]

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

Marking both of these as "changes requested" to highlight the similarity of 
issues in them.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55044



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

[PATCH] D55125: [clang-tidy] Fix a false positive in misc-redundant-expression check

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

Szelethus added a comment.

Yup, I agree with everything that was said.


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

https://reviews.llvm.org/D55125



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

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

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In D55044#1312438 , @lebedev.ri wrote:

> Thanks for working on this.
>  Semi-duplicate of D50852  Please see 
> discussion there.
>  It should not be an abseil-specific check, the prefix (`std`,`abseil`), and 
> the function (`make_unique`) should be a config option, and it should likely 
> be a part of `modernize-use-auto`.


I agree here. Instead of reimplementing the functionality we should rather 
refactor the existing check and make it flexible. It is possible to register 
the `absl` version of it with a different default-configuration so the 
refactoring will be the correct one.




Comment at: test/clang-tidy/abseil-make-unique.cpp:43
+
+  std::unique_ptr b;
+  b.reset(new int(2));

All your test cases seem to create only a single `unique_ptr`, please add cases 
like

```
std::unique_ptr b1(new int(32)), b2(new int(42));
```
From my experience in rewriting decls, they should be excluded. It is very 
cumbersome to get it right in all cases (think pointers, pointer to pointers, 
const, ...) and `readability-isolcate-declaration` exists to split variable 
declarations first.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55044



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

[PATCH] D54737: [clang-tidy] Add the abseil-duration-comparison check

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In D54737#1314946 , @hwright wrote:

> Oh, and thanks for taking the time to review this. :)


No problem!
I will commit on Monday evening (Europe evening) if there are no further 
comments from other reviewers.
Thank you for the patch!


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

https://reviews.llvm.org/D54737



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

[PATCH] D55125: [clang-tidy] Fix a false positive in misc-redundant-expression check

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

> Wiring out the lexical comparison and AST based comparison sounds like an 
> interesting idea, however IMHO such a setting is too coarse-grained on the 
> file level.  My guess would be that it depends from expression (fragment) to 
> expression fragment how you want to compare them: for macros lexical 
> comparison is better, for arithmetic expressions with many parentheses AST 
> based recursive comparison may be a better fit (as implemented also in this 
> checker).

Yes, I agree. I think having such a utility would make sense as an future 
addition, but I would not like to block on it for several reasons:

- do we know how often it this case even appears?
- is it a real false positive in all cases?
- it can be silence with `// NOLINT` if it actually is a false-positive

My gut feeling is that its rather a corner-case (this late patch to 
misc-redundant-expression is somewhat indicative for it) and would block the 
patch from landing.
Allowing future room for flexibility is certainly good.




Comment at: test/clang-tidy/misc-redundant-expression.cpp:109
 #define COND_OP_OTHER_MACRO 9
+#define COND_OP_THIRD_MACRO COND_OP_MACRO
 int TestConditional(int x, int y) {

dkrupp wrote:
> JonasToth wrote:
> > Could you please add a test where the macro is `undef`ed and redefined to 
> > something else?
> I am not sure what you exactly suggest here. It should not matter what 
> COND_OP_THIRD_MACRO is defined to as we lexically compare tokens as they are 
> written in the original source code.
> 
> Could you be a bit more specific what test case you would like to add? 
*should* not matter is my point, please show that :)


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

https://reviews.llvm.org/D55125



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

[PATCH] D54905: [AddressSanitizer] Add flag to disable linking with CXX runtime

[Eugene Leviant via Phabricator via cfe-commits]

evgeny777 added a comment.

>   don't think so. It's just symbol interposition. Every module in the process 
> gets the same definition.

If I have two weak symbols in lib1.a and lib2.a and doesn't define strong one, 
which one will be chosen at link time?


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

https://reviews.llvm.org/D54905



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