[PATCH] D36806: Switch to cantFail(), since it does the same assertion.

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

LGTM...


https://reviews.llvm.org/D36806



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

[PATCH] D36347: Add new script to launch lldb and set breakpoints for diagnostics all diagnostics seen.

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In https://reviews.llvm.org/D36347#901885, @zturner wrote:

> One possible reason for why this never got any traction is that 
> `lldb-commits` wasn't added as a subscriber.  While it's true that the tagged 
> people should have chimed in, having the whole commits list will get some 
> more visibility.  I never saw this come to my inbox.
>
> I think this would be most suitable in the `lldb/examples` folder.
>
> I can't really review this thoroughly, because it relies on a bash script, 
> and I use Windows where we bash isn't really a thing.  My bash is rusty, but 
> it looks like you're embedding a python script in the bash script?  It might 
> be good if this were just an lldb script command.  Take a look at `command 
> script add` in LLDB, and in the `examples` folder for some examples of 
> existing commands that work this way.  The nice thing about doing it this way 
> is that you could just be inside LLDB and write `(lldb) break-diag 
> -Wcovered-switch`, for example, which would be a much tighter integration 
> with the debugger.


Thanks for taking a look.

I mainly work on *nix systems, hence the initial shell script, but if there's 
an interest, I'll be happy to convert it to a single python script as you 
suggest, and resubmit it as a patch to lldb.

Thanks again...


https://reviews.llvm.org/D36347



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

[PATCH] D36347: Add new script to launch lldb and set breakpoints for diagnostics all diagnostics seen.

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

Thanks for all the feedback.  I'll report back once I've addressed all your 
suggestions.

Thanks again...


https://reviews.llvm.org/D36347



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

[PATCH] D47912: [CMake] Consider LLVM_APPEND_VC_REV when generating SVNVersion.inc

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

Sorry for the late comment...

Are you amending an llvm commit or a clang commit?  The reason I ask is that if 
I amend an llvm commit, I have to update around 78 targets (llvm + clang + 
libcxx + libcxxabi + libunwind), but if I amend a clang commit, I only have to 
update 18 targets.

Since tools/clang/lib/Basic/SVNVersion.inc only includes revision number and 
repository for clang and llvm, the contents don't change on 'git commit 
--amend' of the clang repo.  Conversely, include/llvm/Support/VCSRevision.h 
includes the git hash, so it changes each time 'git commit --amend' is run on 
the llvm repo.

If the clang version is the one causing your problem, perhaps you could add a 
check to see if the file actually changed before overwriting it.  See 
llvm/include/llvm/Support/CMakeLists.txt for an example of how to do this, 
i.e., the "undef" case.


Repository:
  rC Clang

https://reviews.llvm.org/D47912



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

[PATCH] D17215: [Sema] Fix PR14211 Crash for explicit instantiation of overloaded template function within class template

[don hinton via Phabricator via cfe-commits]

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

Fixes PR14211.

Since getMostSpecialized only handles FunctionTemplateDecl matches,
keep track of non-FunctionTemplateDecl matches and only call
getMostSpecialized if no non-FunctionTemplateDecl matches are found.

Added tests.


https://reviews.llvm.org/D17215

Files:
  lib/Sema/SemaTemplate.cpp
  test/CXX/temp/temp.decls/temp.mem/p5.cpp


Index: test/CXX/temp/temp.decls/temp.mem/p5.cpp
===
--- test/CXX/temp/temp.decls/temp.mem/p5.cpp
+++ test/CXX/temp/temp.decls/temp.mem/p5.cpp
@@ -77,3 +77,16 @@
   x0.operator float *();
   x0c.operator const char*();
 }
+
+namespace PR14211 {
+template  struct X {
+  void foo(U){}
+  template  void foo(T){}
+
+  template  void bar(T){}
+  void bar(U){}
+};
+
+template void X::foo(int);
+template void X::bar(int);
+}
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -8956,6 +8956,7 @@
   //  instantiated from the member definition associated with its class
   //  template.
   UnresolvedSet<8> Matches;
+  FunctionDecl *NonTemplateMatch = nullptr;
   AttributeList *Attr = D.getDeclSpec().getAttributes().getList();
   TemplateSpecCandidateSet FailedCandidates(D.getIdentifierLoc());
   for (LookupResult::iterator P = Previous.begin(), PEnd = Previous.end();
@@ -8966,12 +8967,22 @@
 QualType Adjusted = adjustCCAndNoReturn(R, Method->getType(),
 /*AdjustExceptionSpec*/true);
 if (Context.hasSameUnqualifiedType(Method->getType(), Adjusted)) {
-  Matches.clear();
-
+  if (!Method->getPrimaryTemplate()) {
+// FIXME: Can this assert ever happen?  Needs a test.
+assert(!NonTemplateMatch && "Multiple NonTemplateMatches");
+NonTemplateMatch = Method;
+continue;
+  }
   Matches.addDecl(Method, P.getAccess());
+
+  // FIXME: Can this ever be true?  Even so, should it really be a
+  // break?  Needs a test.
   if (Method->getTemplateSpecializationKind() == TSK_Undeclared)
 break;
+  continue;
 }
+// FIXME:  Would this be considered an almost match as well?
+continue;
   }
 }
 
@@ -9012,19 +9023,22 @@
 Matches.addDecl(Specialization, P.getAccess());
   }
 
-  // Find the most specialized function template specialization.
-  UnresolvedSetIterator Result = getMostSpecialized(
-  Matches.begin(), Matches.end(), FailedCandidates,
-  D.getIdentifierLoc(),
-  PDiag(diag::err_explicit_instantiation_not_known) << Name,
-  PDiag(diag::err_explicit_instantiation_ambiguous) << Name,
-  PDiag(diag::note_explicit_instantiation_candidate));
-
-  if (Result == Matches.end())
-return true;
+  FunctionDecl *Specialization = NonTemplateMatch;
+  if (!Specialization) {
+// Find the most specialized function template specialization.
+UnresolvedSetIterator Result = getMostSpecialized(
+Matches.begin(), Matches.end(), FailedCandidates,
+D.getIdentifierLoc(),
+PDiag(diag::err_explicit_instantiation_not_known) << Name,
+PDiag(diag::err_explicit_instantiation_ambiguous) << Name,
+PDiag(diag::note_explicit_instantiation_candidate));
+
+if (Result == Matches.end())
+  return true;
 
-  // Ignore access control bits, we don't need them for redeclaration checking.
-  FunctionDecl *Specialization = cast(*Result);
+// Ignore access control bits, we don't need them for redeclaration 
checking.
+Specialization = cast(*Result);
+  }
 
   // C++11 [except.spec]p4
   // In an explicit instantiation an exception-specification may be specified,


Index: test/CXX/temp/temp.decls/temp.mem/p5.cpp
===
--- test/CXX/temp/temp.decls/temp.mem/p5.cpp
+++ test/CXX/temp/temp.decls/temp.mem/p5.cpp
@@ -77,3 +77,16 @@
   x0.operator float *();
   x0c.operator const char*();
 }
+
+namespace PR14211 {
+template  struct X {
+  void foo(U){}
+  template  void foo(T){}
+
+  template  void bar(T){}
+  void bar(U){}
+};
+
+template void X::foo(int);
+template void X::bar(int);
+}
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -8956,6 +8956,7 @@
   //  instantiated from the member definition associated with its class
   //  template.
   UnresolvedSet<8> Matches;
+  FunctionDecl *NonTemplateMatch = nullptr;
   AttributeList *Attr = D.getDeclSpec().getAttributes().getList();
   TemplateSpecCandidateSet FailedCandidates(D.getIdentifierLoc());
   for (LookupResult::iterator P = Previous.begin(), PEnd = Previous.end();
@@ -8966,12 +8967,22 @@
 QualType Adjusted = adjustCCAndNoReturn(R, Method->getType(),
   

[PATCH] D17215: [Sema] Fix PR14211 Crash for explicit instantiation of overloaded template function within class template

[don hinton via Phabricator via cfe-commits]

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

- Addressed comments.


https://reviews.llvm.org/D17215

Files:
  lib/Sema/SemaTemplate.cpp
  test/CXX/temp/temp.decls/temp.mem/p5.cpp


Index: test/CXX/temp/temp.decls/temp.mem/p5.cpp
===
--- test/CXX/temp/temp.decls/temp.mem/p5.cpp
+++ test/CXX/temp/temp.decls/temp.mem/p5.cpp
@@ -77,3 +77,16 @@
   x0.operator float *();
   x0c.operator const char*();
 }
+
+namespace PR14211 {
+template  struct X {
+  void foo(U){}
+  template  void foo(T){}
+
+  template  void bar(T){}
+  void bar(U){}
+};
+
+template void X::foo(int);
+template void X::bar(int);
+}
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -8955,7 +8955,8 @@
   //   A member function [...] of a class template can be explicitly
   //  instantiated from the member definition associated with its class
   //  template.
-  UnresolvedSet<8> Matches;
+  UnresolvedSet<8> TemplateMatches;
+  FunctionDecl *NonTemplateMatch = nullptr;
   AttributeList *Attr = D.getDeclSpec().getAttributes().getList();
   TemplateSpecCandidateSet FailedCandidates(D.getIdentifierLoc());
   for (LookupResult::iterator P = Previous.begin(), PEnd = Previous.end();
@@ -8966,11 +8967,13 @@
 QualType Adjusted = adjustCCAndNoReturn(R, Method->getType(),
 /*AdjustExceptionSpec*/true);
 if (Context.hasSameUnqualifiedType(Method->getType(), Adjusted)) {
-  Matches.clear();
-
-  Matches.addDecl(Method, P.getAccess());
-  if (Method->getTemplateSpecializationKind() == TSK_Undeclared)
-break;
+  if (Method->getPrimaryTemplate()) {
+TemplateMatches.addDecl(Method, P.getAccess());
+  } else {
+// FIXME: Can this assert ever happen?  Needs a test.
+assert(!NonTemplateMatch && "Multiple NonTemplateMatches");
+NonTemplateMatch = Method;
+  }
 }
   }
 }
@@ -9009,22 +9012,25 @@
   continue;
 }
 
-Matches.addDecl(Specialization, P.getAccess());
+TemplateMatches.addDecl(Specialization, P.getAccess());
   }
 
-  // Find the most specialized function template specialization.
-  UnresolvedSetIterator Result = getMostSpecialized(
-  Matches.begin(), Matches.end(), FailedCandidates,
-  D.getIdentifierLoc(),
-  PDiag(diag::err_explicit_instantiation_not_known) << Name,
-  PDiag(diag::err_explicit_instantiation_ambiguous) << Name,
-  PDiag(diag::note_explicit_instantiation_candidate));
-
-  if (Result == Matches.end())
-return true;
+  FunctionDecl *Specialization = NonTemplateMatch;
+  if (!Specialization) {
+// Find the most specialized function template specialization.
+UnresolvedSetIterator Result = getMostSpecialized(
+TemplateMatches.begin(), TemplateMatches.end(), FailedCandidates,
+D.getIdentifierLoc(),
+PDiag(diag::err_explicit_instantiation_not_known) << Name,
+PDiag(diag::err_explicit_instantiation_ambiguous) << Name,
+PDiag(diag::note_explicit_instantiation_candidate));
+
+if (Result == TemplateMatches.end())
+  return true;
 
-  // Ignore access control bits, we don't need them for redeclaration checking.
-  FunctionDecl *Specialization = cast(*Result);
+// Ignore access control bits, we don't need them for redeclaration 
checking.
+Specialization = cast(*Result);
+  }
 
   // C++11 [except.spec]p4
   // In an explicit instantiation an exception-specification may be specified,


Index: test/CXX/temp/temp.decls/temp.mem/p5.cpp
===
--- test/CXX/temp/temp.decls/temp.mem/p5.cpp
+++ test/CXX/temp/temp.decls/temp.mem/p5.cpp
@@ -77,3 +77,16 @@
   x0.operator float *();
   x0c.operator const char*();
 }
+
+namespace PR14211 {
+template  struct X {
+  void foo(U){}
+  template  void foo(T){}
+
+  template  void bar(T){}
+  void bar(U){}
+};
+
+template void X::foo(int);
+template void X::bar(int);
+}
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -8955,7 +8955,8 @@
   //   A member function [...] of a class template can be explicitly
   //  instantiated from the member definition associated with its class
   //  template.
-  UnresolvedSet<8> Matches;
+  UnresolvedSet<8> TemplateMatches;
+  FunctionDecl *NonTemplateMatch = nullptr;
   AttributeList *Attr = D.getDeclSpec().getAttributes().getList();
   TemplateSpecCandidateSet FailedCandidates(D.getIdentifierLoc());
   for (LookupResult::iterator P = Previous.begin(), PEnd = Previous.end();
@@ -8966,11 +8967,13 @@
 QualType Adjusted = adjustCCAndNoReturn(R, Method->getType(),
 /*Adjust

[PATCH] D17215: [Sema] Fix PR14211 Crash for explicit instantiation of overloaded template function within class template

[don hinton via Phabricator via cfe-commits]

hintonda added a comment.

Great, thanks John.

Yes, that's correct -- I do not have commit access.


https://reviews.llvm.org/D17215



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

[PATCH] D20693: [clang-tidy] New checker to replace dynamic exception specifications

[don hinton via Phabricator via cfe-commits]

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

Reopening due to test failures on Linux -- was rolled back.


https://reviews.llvm.org/D20693



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

[PATCH] D20693: [clang-tidy] New checker to replace dynamic exception specifications

[don hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 101659.
hintonda added a comment.
Herald added a subscriber: xazax.hun.

In order to fix diagnostic corruption in some of the buildbot tests
(unable to reproduce locally):

- make NoexceptMacro a static variable so it's lifetime doesn't end when 
UseNoexceptCheck is destroyed.

- pass a const char* instead of a StringRef to DiagnosticBuilder so it won't 
create a temporary std::string and cache the address of the temporary char * it 
owns.


https://reviews.llvm.org/D20693

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/UseNoexceptCheck.cpp
  clang-tidy/modernize/UseNoexceptCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-use-noexcept.rst
  test/clang-tidy/modernize-use-noexcept-macro.cpp
  test/clang-tidy/modernize-use-noexcept-opt.cpp
  test/clang-tidy/modernize-use-noexcept.cpp

Index: test/clang-tidy/modernize-use-noexcept.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-noexcept.cpp
@@ -0,0 +1,104 @@
+// RUN: %check_clang_tidy %s modernize-use-noexcept %t -- \
+// RUN:   -- -std=c++11
+
+class A {};
+class B {};
+
+void foo() throw();
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: dynamic exception specification 'throw()' is deprecated; consider using 'noexcept' instead [modernize-use-noexcept]
+// CHECK-FIXES: void foo() noexcept;
+
+template 
+void foo() throw();
+void footest() { foo(); foo(); }
+// CHECK-MESSAGES: :[[@LINE-2]]:12: warning: dynamic exception specification 'throw()' is deprecated; consider using 'noexcept' instead [modernize-use-noexcept]
+// CHECK-FIXES: void foo() noexcept;
+
+void bar() throw(...);
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: dynamic exception specification 'throw(...)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void bar() noexcept(false);
+
+void k() throw(int(int));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: dynamic exception specification 'throw(int(int))' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void k() noexcept(false);
+
+void foobar() throw(A, B)
+{}
+// CHECK-MESSAGES: :[[@LINE-2]]:15: warning: dynamic exception specification 'throw(A, B)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void foobar() noexcept(false)
+
+void baz(int = (throw A(), 0)) throw(A, B) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: dynamic exception specification 'throw(A, B)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void baz(int = (throw A(), 0)) noexcept(false) {}
+
+void g(void (*fp)(void) throw());
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: dynamic exception specification 'throw()' is deprecated; consider using 'noexcept' instead [modernize-use-noexcept]
+// CHECK-FIXES: void g(void (*fp)(void) noexcept);
+
+void f(void (*fp)(void) throw(int)) throw(char);
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: dynamic exception specification 'throw(int)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-MESSAGES: :[[@LINE-2]]:37: warning: dynamic exception specification 'throw(char)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void f(void (*fp)(void) noexcept(false)) noexcept(false);
+
+#define THROW throw
+void h(void (*fp)(void) THROW(int)) THROW(char);
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: dynamic exception specification 'THROW(int)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-MESSAGES: :[[@LINE-2]]:37: warning: dynamic exception specification 'THROW(char)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void h(void (*fp)(void) noexcept(false)) noexcept(false);
+
+void j() throw(int(int) throw(void(void) throw(int)));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: dynamic exception specification 'throw(int(int) throw(void(void) throw(int)))' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void j() noexcept(false);
+
+class Y {
+  Y() throw() = default;
+};
+// CHECK-MESSAGES: :[[@LINE-2]]:7: warning: dynamic exception specification 'throw()' is deprecated; consider using 'noexcept' instead [modernize-use-noexcept]
+// CHECK-FIXES: Y() noexcept = default;
+
+struct Z {
+  void operator delete(void *ptr) throw();
+  void operator delete[](void *ptr) throw(int);
+  ~Z() throw(int) {}
+};
+// CHECK-MESSAGES: :[[@LINE-4]]:35: warning: dynamic exception specification 'throw()' is deprecated; consider using 'noexcept' instead [modernize-use-noexcept]
+// CHECK-MESSAGES: :[[@LINE-4]]:37: warning: dynamic exception specification 'throw(int)' is deprecated; consider using 'noexcept(false)' instead [

[PATCH] D17215: [Sema] Fix PR14211 Crash for explicit instantiation of overloaded template function within class template

[don hinton via Phabricator via cfe-commits]

hintonda added a comment.

Since I don't have commit access, could you commit this for me?

thanks...
don


https://reviews.llvm.org/D17215



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

[PATCH] D20693: [clang-tidy] New checker to replace dynamic exception specifications

[don hinton via Phabricator via cfe-commits]

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

- Rollback to previous version: rebased + r293218 and r293234.


https://reviews.llvm.org/D20693

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/UseNoexceptCheck.cpp
  clang-tidy/modernize/UseNoexceptCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-use-noexcept.rst
  test/clang-tidy/modernize-use-noexcept-macro.cpp
  test/clang-tidy/modernize-use-noexcept-opt.cpp
  test/clang-tidy/modernize-use-noexcept.cpp

Index: test/clang-tidy/modernize-use-noexcept.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-noexcept.cpp
@@ -0,0 +1,104 @@
+// RUN: %check_clang_tidy %s modernize-use-noexcept %t -- \
+// RUN:   -- -std=c++11
+
+class A {};
+class B {};
+
+void foo() throw();
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: dynamic exception specification 'throw()' is deprecated; consider using 'noexcept' instead [modernize-use-noexcept]
+// CHECK-FIXES: void foo() noexcept;
+
+template 
+void foo() throw();
+void footest() { foo(); foo(); }
+// CHECK-MESSAGES: :[[@LINE-2]]:12: warning: dynamic exception specification 'throw()' is deprecated; consider using 'noexcept' instead [modernize-use-noexcept]
+// CHECK-FIXES: void foo() noexcept;
+
+void bar() throw(...);
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: dynamic exception specification 'throw(...)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void bar() noexcept(false);
+
+void k() throw(int(int));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: dynamic exception specification 'throw(int(int))' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void k() noexcept(false);
+
+void foobar() throw(A, B)
+{}
+// CHECK-MESSAGES: :[[@LINE-2]]:15: warning: dynamic exception specification 'throw(A, B)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void foobar() noexcept(false)
+
+void baz(int = (throw A(), 0)) throw(A, B) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: dynamic exception specification 'throw(A, B)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void baz(int = (throw A(), 0)) noexcept(false) {}
+
+void g(void (*fp)(void) throw());
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: dynamic exception specification 'throw()' is deprecated; consider using 'noexcept' instead [modernize-use-noexcept]
+// CHECK-FIXES: void g(void (*fp)(void) noexcept);
+
+void f(void (*fp)(void) throw(int)) throw(char);
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: dynamic exception specification 'throw(int)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-MESSAGES: :[[@LINE-2]]:37: warning: dynamic exception specification 'throw(char)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void f(void (*fp)(void) noexcept(false)) noexcept(false);
+
+#define THROW throw
+void h(void (*fp)(void) THROW(int)) THROW(char);
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: dynamic exception specification 'THROW(int)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-MESSAGES: :[[@LINE-2]]:37: warning: dynamic exception specification 'THROW(char)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void h(void (*fp)(void) noexcept(false)) noexcept(false);
+
+void j() throw(int(int) throw(void(void) throw(int)));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: dynamic exception specification 'throw(int(int) throw(void(void) throw(int)))' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void j() noexcept(false);
+
+class Y {
+  Y() throw() = default;
+};
+// CHECK-MESSAGES: :[[@LINE-2]]:7: warning: dynamic exception specification 'throw()' is deprecated; consider using 'noexcept' instead [modernize-use-noexcept]
+// CHECK-FIXES: Y() noexcept = default;
+
+struct Z {
+  void operator delete(void *ptr) throw();
+  void operator delete[](void *ptr) throw(int);
+  ~Z() throw(int) {}
+};
+// CHECK-MESSAGES: :[[@LINE-4]]:35: warning: dynamic exception specification 'throw()' is deprecated; consider using 'noexcept' instead [modernize-use-noexcept]
+// CHECK-MESSAGES: :[[@LINE-4]]:37: warning: dynamic exception specification 'throw(int)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-MESSAGES: :[[@LINE-4]]:8: warning: dynamic exception specification 'throw(int)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void operator delete(void *ptr) noexcept;
+// CHECK-FIXES: void operator delete[](void *ptr) noexcept(false);
+// CHECK-FIXES: ~Z() no

[PATCH] D20693: [clang-tidy] New checker to replace dynamic exception specifications

[don hinton via Phabricator via cfe-commits]

hintonda added a comment.

Unfortunately, the logs are no longer available, and I don't have copies.

However, I think I know what's going on, so I'll try to submit a fix later 
today.


https://reviews.llvm.org/D20693



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

[PATCH] D20693: [clang-tidy] New checker to replace dynamic exception specifications

[don hinton via Phabricator via cfe-commits]

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



- Only pass %2 parameter if %2 is included in format.


https://reviews.llvm.org/D20693

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/UseNoexceptCheck.cpp
  clang-tidy/modernize/UseNoexceptCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-use-noexcept.rst
  test/clang-tidy/modernize-use-noexcept-macro.cpp
  test/clang-tidy/modernize-use-noexcept-opt.cpp
  test/clang-tidy/modernize-use-noexcept.cpp

Index: test/clang-tidy/modernize-use-noexcept.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-noexcept.cpp
@@ -0,0 +1,104 @@
+// RUN: %check_clang_tidy %s modernize-use-noexcept %t -- \
+// RUN:   -- -std=c++11
+
+class A {};
+class B {};
+
+void foo() throw();
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: dynamic exception specification 'throw()' is deprecated; consider using 'noexcept' instead [modernize-use-noexcept]
+// CHECK-FIXES: void foo() noexcept;
+
+template 
+void foo() throw();
+void footest() { foo(); foo(); }
+// CHECK-MESSAGES: :[[@LINE-2]]:12: warning: dynamic exception specification 'throw()' is deprecated; consider using 'noexcept' instead [modernize-use-noexcept]
+// CHECK-FIXES: void foo() noexcept;
+
+void bar() throw(...);
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: dynamic exception specification 'throw(...)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void bar() noexcept(false);
+
+void k() throw(int(int));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: dynamic exception specification 'throw(int(int))' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void k() noexcept(false);
+
+void foobar() throw(A, B)
+{}
+// CHECK-MESSAGES: :[[@LINE-2]]:15: warning: dynamic exception specification 'throw(A, B)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void foobar() noexcept(false)
+
+void baz(int = (throw A(), 0)) throw(A, B) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: dynamic exception specification 'throw(A, B)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void baz(int = (throw A(), 0)) noexcept(false) {}
+
+void g(void (*fp)(void) throw());
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: dynamic exception specification 'throw()' is deprecated; consider using 'noexcept' instead [modernize-use-noexcept]
+// CHECK-FIXES: void g(void (*fp)(void) noexcept);
+
+void f(void (*fp)(void) throw(int)) throw(char);
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: dynamic exception specification 'throw(int)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-MESSAGES: :[[@LINE-2]]:37: warning: dynamic exception specification 'throw(char)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void f(void (*fp)(void) noexcept(false)) noexcept(false);
+
+#define THROW throw
+void h(void (*fp)(void) THROW(int)) THROW(char);
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: dynamic exception specification 'THROW(int)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-MESSAGES: :[[@LINE-2]]:37: warning: dynamic exception specification 'THROW(char)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void h(void (*fp)(void) noexcept(false)) noexcept(false);
+
+void j() throw(int(int) throw(void(void) throw(int)));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: dynamic exception specification 'throw(int(int) throw(void(void) throw(int)))' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void j() noexcept(false);
+
+class Y {
+  Y() throw() = default;
+};
+// CHECK-MESSAGES: :[[@LINE-2]]:7: warning: dynamic exception specification 'throw()' is deprecated; consider using 'noexcept' instead [modernize-use-noexcept]
+// CHECK-FIXES: Y() noexcept = default;
+
+struct Z {
+  void operator delete(void *ptr) throw();
+  void operator delete[](void *ptr) throw(int);
+  ~Z() throw(int) {}
+};
+// CHECK-MESSAGES: :[[@LINE-4]]:35: warning: dynamic exception specification 'throw()' is deprecated; consider using 'noexcept' instead [modernize-use-noexcept]
+// CHECK-MESSAGES: :[[@LINE-4]]:37: warning: dynamic exception specification 'throw(int)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-MESSAGES: :[[@LINE-4]]:8: warning: dynamic exception specification 'throw(int)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void operator delete(void *ptr) noexcept;
+// CHECK-FIXES: void operator delete[](void *ptr) noexcept(false);
+// CHECK-FIXES: ~Z() noexcept(

[PATCH] D20693: [clang-tidy] New checker to replace dynamic exception specifications

[don hinton via Phabricator via cfe-commits]

hintonda added a comment.

I have not, as yet, been able to reproduce the buildbot failures.  They were 
essentially intermittent seg-faults, and corrupt diag output.

I will work on creating a test that can reproduce the problem.


https://reviews.llvm.org/D20693



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

[PATCH] D20693: [clang-tidy] New checker to replace dynamic exception specifications

[don hinton via Phabricator via cfe-commits]

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

- Rollback last change.


https://reviews.llvm.org/D20693

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/UseNoexceptCheck.cpp
  clang-tidy/modernize/UseNoexceptCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-use-noexcept.rst
  test/clang-tidy/modernize-use-noexcept-macro.cpp
  test/clang-tidy/modernize-use-noexcept-opt.cpp
  test/clang-tidy/modernize-use-noexcept.cpp

Index: test/clang-tidy/modernize-use-noexcept.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-noexcept.cpp
@@ -0,0 +1,104 @@
+// RUN: %check_clang_tidy %s modernize-use-noexcept %t -- \
+// RUN:   -- -std=c++11
+
+class A {};
+class B {};
+
+void foo() throw();
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: dynamic exception specification 'throw()' is deprecated; consider using 'noexcept' instead [modernize-use-noexcept]
+// CHECK-FIXES: void foo() noexcept;
+
+template 
+void foo() throw();
+void footest() { foo(); foo(); }
+// CHECK-MESSAGES: :[[@LINE-2]]:12: warning: dynamic exception specification 'throw()' is deprecated; consider using 'noexcept' instead [modernize-use-noexcept]
+// CHECK-FIXES: void foo() noexcept;
+
+void bar() throw(...);
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: dynamic exception specification 'throw(...)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void bar() noexcept(false);
+
+void k() throw(int(int));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: dynamic exception specification 'throw(int(int))' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void k() noexcept(false);
+
+void foobar() throw(A, B)
+{}
+// CHECK-MESSAGES: :[[@LINE-2]]:15: warning: dynamic exception specification 'throw(A, B)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void foobar() noexcept(false)
+
+void baz(int = (throw A(), 0)) throw(A, B) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: dynamic exception specification 'throw(A, B)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void baz(int = (throw A(), 0)) noexcept(false) {}
+
+void g(void (*fp)(void) throw());
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: dynamic exception specification 'throw()' is deprecated; consider using 'noexcept' instead [modernize-use-noexcept]
+// CHECK-FIXES: void g(void (*fp)(void) noexcept);
+
+void f(void (*fp)(void) throw(int)) throw(char);
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: dynamic exception specification 'throw(int)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-MESSAGES: :[[@LINE-2]]:37: warning: dynamic exception specification 'throw(char)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void f(void (*fp)(void) noexcept(false)) noexcept(false);
+
+#define THROW throw
+void h(void (*fp)(void) THROW(int)) THROW(char);
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: dynamic exception specification 'THROW(int)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-MESSAGES: :[[@LINE-2]]:37: warning: dynamic exception specification 'THROW(char)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void h(void (*fp)(void) noexcept(false)) noexcept(false);
+
+void j() throw(int(int) throw(void(void) throw(int)));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: dynamic exception specification 'throw(int(int) throw(void(void) throw(int)))' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void j() noexcept(false);
+
+class Y {
+  Y() throw() = default;
+};
+// CHECK-MESSAGES: :[[@LINE-2]]:7: warning: dynamic exception specification 'throw()' is deprecated; consider using 'noexcept' instead [modernize-use-noexcept]
+// CHECK-FIXES: Y() noexcept = default;
+
+struct Z {
+  void operator delete(void *ptr) throw();
+  void operator delete[](void *ptr) throw(int);
+  ~Z() throw(int) {}
+};
+// CHECK-MESSAGES: :[[@LINE-4]]:35: warning: dynamic exception specification 'throw()' is deprecated; consider using 'noexcept' instead [modernize-use-noexcept]
+// CHECK-MESSAGES: :[[@LINE-4]]:37: warning: dynamic exception specification 'throw(int)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-MESSAGES: :[[@LINE-4]]:8: warning: dynamic exception specification 'throw(int)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void operator delete(void *ptr) noexcept;
+// CHECK-FIXES: void operator delete[](void *ptr) noexcept(false);
+// CHECK-FIXES: ~Z() noexcept(false) {}
+
+struct S {
+  void 

[PATCH] D20693: [clang-tidy] New checker to replace dynamic exception specifications

[don hinton via Phabricator via cfe-commits]

hintonda added a comment.

Just ran asan on linux and we have a heap-use-after-free in the std::string 
ctor.

Here's a partial stack dump:

4980==ERROR: AddressSanitizer: heap-use-after-free on address 0x60424328 at 
pc 0x0057ad32 bp 0x7ffd240a7f50 sp 0x7ffd240a7700
-

READ of size 8 at 0x60424328 thread T0

  #0 0x57ad31 in __interceptor_memcpy.part.36 
/home/d80049854/projects/clang/4.0.0/llvm/projects/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc:655
  #1 0x6c5960 in char* std::string::_S_construct(char const*, char 
const*, std::allocator const&, std::forward_iterator_tag) 
/usr/lib/gcc/x86_64-linux-gnu/6.2.0/../../../../include/c+

+/6.2.0/bits/basic_string.tcc:580:6

  #2 0x7fe7bd7bc98a in std::basic_string, 
std::allocator >::basic_string(char const*, unsigned long, 
std::allocator const&) (/usr/lib/x86_64-linux-gnu/libstdc++.

so.6+0xc598a)

  #3 0x67b89f in llvm::StringRef::str() const 
/home/d80049854/projects/clang/llvm/include/llvm/ADT/StringRef.h:230:14
  #4 0x67b3dd in llvm::StringRef::operator std::string() const 
/home/d80049854/projects/clang/llvm/include/llvm/ADT/StringRef.h:257:14
  #5 0xd679d2 in clang::FixItHint::CreateReplacement(clang::CharSourceRange, 
llvm::StringRef) 
/home/d80049854/projects/clang/llvm/tools/clang/include/clang/Basic/Diagnostic.h:131:25
  #6 0x1213006 in 
clang::tidy::modernize::UseNoexceptCheck::check(clang::ast_matchers::MatchFinder::MatchResult
 const&) 
/home/d80049854/projects/clang/llvm/tools/clang/tools/extra/clang-tidy/modernize/U

$ ../../4.0.0/build/Release/bin/clang -v
clang version 4.0.0 (tags/RELEASE_400/final)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: 
/home/d80049854/projects/clang/build/Debug/../../4.0.0/build/Release/bin
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/4.6
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/4.6.4
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/4.8
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/4.8.4
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/4.9
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/4.9.3
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/5
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/5.4.1
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/6
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/6.2.0
Selected GCC installation: /usr/lib/gcc/x86_64-linux-gnu/6.2.0
Candidate multilib: .;@m64
Selected multilib: .;@m64

stdlibc++ from gcc 6.2:

$ g++-6 -v
Using built-in specs.
COLLECT_GCC=g++-6
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/6/lto-wrapper
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu 
6.2.0-3ubuntu11~14.04' --with-bugurl=file:///usr/share/doc/gcc-6/README.Bugs 
--enable-languages=c,ada,c++,java,go,d,fortran,objc,obj-c++ --prefix=/usr 
--program-suffix=-6 --enable-shared --enable-linker-build-id 
--libexecdir=/usr/lib --without-included-gettext --enable-threads=posix 
--libdir=/usr/lib --enable-nls --with-sysroot=/ --enable-clocale=gnu 
--enable-libstdcxx-debug --enable-libstdcxx-time=yes 
--with-default-libstdcxx-abi=gcc4-compatible --disable-libstdcxx-dual-abi 
--enable-gnu-unique-object --disable-vtable-verify --enable-libmpx 
--enable-plugin --with-system-zlib --disable-browser-plugin 
--enable-java-awt=gtk --enable-gtk-cairo 
--with-java-home=/usr/lib/jvm/java-1.5.0-gcj-6-amd64/jre --enable-java-home 
--with-jvm-root-dir=/usr/lib/jvm/java-1.5.0-gcj-6-amd64 
--with-jvm-jar-dir=/usr/lib/jvm-exports/java-1.5.0-gcj-6-amd64 
--with-arch-directory=amd64 --with-ecj-jar=/usr/share/java/eclipse-ecj.jar 
--enable-objc-gc --enable-multiarch --disable-werror --with-arch-32=i686 
--with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib 
--with-tune=generic --enable-checking=release --build=x86_64-linux-gnu 
--host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
gcc version 6.2.0 20160901 (Ubuntu 6.2.0-3ubuntu11~14.04)

$ ldd bin/clang-tidy

  linux-vdso.so.1 =>  (0x7ffde1996000)
  libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x7f6104f2a000)
  librt.so.1 => /lib/x86_64-linux-gnu/librt.so.1 (0x7f6104d22000)
  libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x7f6104b1e000)
  libtinfo.so.5 => /lib/x86_64-linux-gnu/libtinfo.so.5 (0x7f61048f5000)
  libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x7f61045ef000)
  libstdc++.so.6 => /usr/lib/x86_64-linux-gnu/libstdc++.so.6 
(0x7f61042dd000)
  libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x7f61040c6000)
  libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x7f6103d01000)
  /lib64/ld-linux-x86-64.so.2 (0x7f6105148000)


https://reviews.llvm.org/D20693



___

[PATCH] D20693: [clang-tidy] New checker to replace dynamic exception specifications

[don hinton via Phabricator via cfe-commits]

hintonda added a comment.

btw, here's how I built it, in case that matters...

CC=../../4.0.0/build/Release/bin/clang 
CXX=../../4.0.0/build/Release/bin/clang++  \
 cmake ../../llvm/ \
 -GNinja \
 -DLLVM_USE_SANITIZER=Address \
 -DCMAKE_BUILD_TYPE=Debug \
 -DLLVM_TARGETS_TO_BUILD="X86" \
 -DLLVM_PARALLEL_LINK_JOBS=4


https://reviews.llvm.org/D20693



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

[PATCH] D20693: [clang-tidy] New checker to replace dynamic exception specifications

[don hinton via Phabricator via cfe-commits]

hintonda added a comment.

Here's a simple example that demonstrates the corruption I'm seeing:

#include "llvm/ADT/StringRef.h"

int main() {

  std::string ss = "";
  llvm::StringRef Ref = true ? "noexcept" : ss;
  std::string s = Ref;
  return 0;

}


https://reviews.llvm.org/D20693



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

[PATCH] D20693: [clang-tidy] New checker to replace dynamic exception specifications

[don hinton via Phabricator via cfe-commits]

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

- Make sure types for ternary operator are the same.


https://reviews.llvm.org/D20693

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/UseNoexceptCheck.cpp
  clang-tidy/modernize/UseNoexceptCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-use-noexcept.rst
  test/clang-tidy/modernize-use-noexcept-macro.cpp
  test/clang-tidy/modernize-use-noexcept-opt.cpp
  test/clang-tidy/modernize-use-noexcept.cpp

Index: test/clang-tidy/modernize-use-noexcept.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-noexcept.cpp
@@ -0,0 +1,104 @@
+// RUN: %check_clang_tidy %s modernize-use-noexcept %t -- \
+// RUN:   -- -std=c++11
+
+class A {};
+class B {};
+
+void foo() throw();
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: dynamic exception specification 'throw()' is deprecated; consider using 'noexcept' instead [modernize-use-noexcept]
+// CHECK-FIXES: void foo() noexcept;
+
+template 
+void foo() throw();
+void footest() { foo(); foo(); }
+// CHECK-MESSAGES: :[[@LINE-2]]:12: warning: dynamic exception specification 'throw()' is deprecated; consider using 'noexcept' instead [modernize-use-noexcept]
+// CHECK-FIXES: void foo() noexcept;
+
+void bar() throw(...);
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: dynamic exception specification 'throw(...)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void bar() noexcept(false);
+
+void k() throw(int(int));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: dynamic exception specification 'throw(int(int))' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void k() noexcept(false);
+
+void foobar() throw(A, B)
+{}
+// CHECK-MESSAGES: :[[@LINE-2]]:15: warning: dynamic exception specification 'throw(A, B)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void foobar() noexcept(false)
+
+void baz(int = (throw A(), 0)) throw(A, B) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: dynamic exception specification 'throw(A, B)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void baz(int = (throw A(), 0)) noexcept(false) {}
+
+void g(void (*fp)(void) throw());
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: dynamic exception specification 'throw()' is deprecated; consider using 'noexcept' instead [modernize-use-noexcept]
+// CHECK-FIXES: void g(void (*fp)(void) noexcept);
+
+void f(void (*fp)(void) throw(int)) throw(char);
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: dynamic exception specification 'throw(int)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-MESSAGES: :[[@LINE-2]]:37: warning: dynamic exception specification 'throw(char)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void f(void (*fp)(void) noexcept(false)) noexcept(false);
+
+#define THROW throw
+void h(void (*fp)(void) THROW(int)) THROW(char);
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: dynamic exception specification 'THROW(int)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-MESSAGES: :[[@LINE-2]]:37: warning: dynamic exception specification 'THROW(char)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void h(void (*fp)(void) noexcept(false)) noexcept(false);
+
+void j() throw(int(int) throw(void(void) throw(int)));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: dynamic exception specification 'throw(int(int) throw(void(void) throw(int)))' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void j() noexcept(false);
+
+class Y {
+  Y() throw() = default;
+};
+// CHECK-MESSAGES: :[[@LINE-2]]:7: warning: dynamic exception specification 'throw()' is deprecated; consider using 'noexcept' instead [modernize-use-noexcept]
+// CHECK-FIXES: Y() noexcept = default;
+
+struct Z {
+  void operator delete(void *ptr) throw();
+  void operator delete[](void *ptr) throw(int);
+  ~Z() throw(int) {}
+};
+// CHECK-MESSAGES: :[[@LINE-4]]:35: warning: dynamic exception specification 'throw()' is deprecated; consider using 'noexcept' instead [modernize-use-noexcept]
+// CHECK-MESSAGES: :[[@LINE-4]]:37: warning: dynamic exception specification 'throw(int)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-MESSAGES: :[[@LINE-4]]:8: warning: dynamic exception specification 'throw(int)' is deprecated; consider using 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void operator delete(void *ptr) noexcept;
+// CHECK-FIXES: void operator delete[](void *ptr) noexcept(false);
+// CHECK-FIXES: ~Z() noexcept(fal

[PATCH] D20693: [clang-tidy] New checker to replace dynamic exception specifications

[don hinton via Phabricator via cfe-commits]

hintonda added a comment.

Great, thanks for you help.

Could you commit this for me?


https://reviews.llvm.org/D20693



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

[PATCH] D17143: [Sema] PR25156 Crash when parsing dtor call on incomplete type

[don hinton via Phabricator via cfe-commits]

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

Rebase and move test to existing file.


https://reviews.llvm.org/D17143

Files:
  lib/Sema/SemaExprCXX.cpp
  test/SemaCXX/nested-name-spec.cpp


Index: test/SemaCXX/nested-name-spec.cpp
===
--- test/SemaCXX/nested-name-spec.cpp
+++ test/SemaCXX/nested-name-spec.cpp
@@ -169,6 +169,13 @@
 struct Y;  // expected-note{{forward declaration of 'Y'}}
 Y::foo y; // expected-error{{incomplete type 'Y' named in nested name 
specifier}}
 
+namespace PR25156 {
+struct Y;  // expected-note{{forward declaration of 'PR25156::Y'}}
+void foo() {
+  Y::~Y(); // expected-error{{incomplete type 'PR25156::Y' named in nested 
name specifier}}
+}
+}
+
 X::X() : a(5) { } // expected-error{{use of undeclared identifier 'X'}}
 
 struct foo_S {
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -189,7 +189,7 @@
 // have one) and, if that fails to find a match, in the scope (if
 // we're allowed to look there).
 Found.clear();
-if (Step == 0 && LookupCtx)
+if (Step == 0 && LookupCtx && !RequireCompleteDeclContext(SS, LookupCtx))
   LookupQualifiedName(Found, LookupCtx);
 else if (Step == 1 && LookInScope && S)
   LookupName(Found, S);


Index: test/SemaCXX/nested-name-spec.cpp
===
--- test/SemaCXX/nested-name-spec.cpp
+++ test/SemaCXX/nested-name-spec.cpp
@@ -169,6 +169,13 @@
 struct Y;  // expected-note{{forward declaration of 'Y'}}
 Y::foo y; // expected-error{{incomplete type 'Y' named in nested name specifier}}
 
+namespace PR25156 {
+struct Y;  // expected-note{{forward declaration of 'PR25156::Y'}}
+void foo() {
+  Y::~Y(); // expected-error{{incomplete type 'PR25156::Y' named in nested name specifier}}
+}
+}
+
 X::X() : a(5) { } // expected-error{{use of undeclared identifier 'X'}}
 
 struct foo_S {
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -189,7 +189,7 @@
 // have one) and, if that fails to find a match, in the scope (if
 // we're allowed to look there).
 Found.clear();
-if (Step == 0 && LookupCtx)
+if (Step == 0 && LookupCtx && !RequireCompleteDeclContext(SS, LookupCtx))
   LookupQualifiedName(Found, LookupCtx);
 else if (Step == 1 && LookInScope && S)
   LookupName(Found, S);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D17143: [Sema] PR25156 Crash when parsing dtor call on incomplete type

[don hinton via Phabricator via cfe-commits]

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

- Addressed comments.


https://reviews.llvm.org/D17143

Files:
  lib/Sema/SemaExprCXX.cpp
  test/SemaCXX/nested-name-spec.cpp


Index: test/SemaCXX/nested-name-spec.cpp
===
--- test/SemaCXX/nested-name-spec.cpp
+++ test/SemaCXX/nested-name-spec.cpp
@@ -169,6 +169,13 @@
 struct Y;  // expected-note{{forward declaration of 'Y'}}
 Y::foo y; // expected-error{{incomplete type 'Y' named in nested name 
specifier}}
 
+namespace PR25156 {
+struct Y;  // expected-note{{forward declaration of 'PR25156::Y'}}
+void foo() {
+  Y::~Y(); // expected-error{{incomplete type 'PR25156::Y' named in nested 
name specifier}}
+}
+}
+
 X::X() : a(5) { } // expected-error{{use of undeclared identifier 'X'}}
 
 struct foo_S {
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -189,12 +189,15 @@
 // have one) and, if that fails to find a match, in the scope (if
 // we're allowed to look there).
 Found.clear();
-if (Step == 0 && LookupCtx)
+if (Step == 0 && LookupCtx) {
+  if (RequireCompleteDeclContext(SS, LookupCtx))
+return nullptr;
   LookupQualifiedName(Found, LookupCtx);
-else if (Step == 1 && LookInScope && S)
+} else if (Step == 1 && LookInScope && S) {
   LookupName(Found, S);
-else
+} else {
   continue;
+}
 
 // FIXME: Should we be suppressing ambiguities here?
 if (Found.isAmbiguous())


Index: test/SemaCXX/nested-name-spec.cpp
===
--- test/SemaCXX/nested-name-spec.cpp
+++ test/SemaCXX/nested-name-spec.cpp
@@ -169,6 +169,13 @@
 struct Y;  // expected-note{{forward declaration of 'Y'}}
 Y::foo y; // expected-error{{incomplete type 'Y' named in nested name specifier}}
 
+namespace PR25156 {
+struct Y;  // expected-note{{forward declaration of 'PR25156::Y'}}
+void foo() {
+  Y::~Y(); // expected-error{{incomplete type 'PR25156::Y' named in nested name specifier}}
+}
+}
+
 X::X() : a(5) { } // expected-error{{use of undeclared identifier 'X'}}
 
 struct foo_S {
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -189,12 +189,15 @@
 // have one) and, if that fails to find a match, in the scope (if
 // we're allowed to look there).
 Found.clear();
-if (Step == 0 && LookupCtx)
+if (Step == 0 && LookupCtx) {
+  if (RequireCompleteDeclContext(SS, LookupCtx))
+return nullptr;
   LookupQualifiedName(Found, LookupCtx);
-else if (Step == 1 && LookInScope && S)
+} else if (Step == 1 && LookInScope && S) {
   LookupName(Found, S);
-else
+} else {
   continue;
+}
 
 // FIXME: Should we be suppressing ambiguities here?
 if (Found.isAmbiguous())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D17143: [Sema] PR25156 Crash when parsing dtor call on incomplete type

[don hinton via Phabricator via cfe-commits]

hintonda closed this revision.
hintonda added a comment.

Committed in r305169.


https://reviews.llvm.org/D17143



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

[PATCH] D16971: [Sema] PR26077 Fixed crash when partial specialization is missing required parameters

[don hinton via Phabricator via cfe-commits]

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

Rebase, fix test, and add FIXME note.


https://reviews.llvm.org/D16971

Files:
  lib/Sema/SemaTemplate.cpp
  test/SemaTemplate/partial-spec-fully-specified.cpp


Index: test/SemaTemplate/partial-spec-fully-specified.cpp
===
--- /dev/null
+++ test/SemaTemplate/partial-spec-fully-specified.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+
+template 
+struct Base{};
+
+template 
+class Test {};
+
+template 
+class Test : Base { // expected-error{{partial specialization 
of 'Test' does not use any of its template parameters}}
+  Test() {}
+};
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -3418,7 +3418,9 @@
 InstantiationDependent)) {
   Diag(TemplateNameLoc, diag::err_partial_spec_fully_specialized)
   << VarTemplate->getDeclName();
-  IsPartialSpecialization = false;
+  // FIXME: Can this ever get triggered?  If so, we need test.  If not,
+  // should this if be removed?
+  return true;
 }
 
 if (isSameAsPrimaryTemplate(VarTemplate->getTemplateParameters(),
@@ -7276,7 +7278,7 @@
 TemplateArgs.arguments(), InstantiationDependent)) {
   Diag(TemplateNameLoc, diag::err_partial_spec_fully_specialized)
 << ClassTemplate->getDeclName();
-  isPartialSpecialization = false;
+  return true;
 }
   }
 


Index: test/SemaTemplate/partial-spec-fully-specified.cpp
===
--- /dev/null
+++ test/SemaTemplate/partial-spec-fully-specified.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+
+template 
+struct Base{};
+
+template 
+class Test {};
+
+template 
+class Test : Base { // expected-error{{partial specialization of 'Test' does not use any of its template parameters}}
+  Test() {}
+};
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -3418,7 +3418,9 @@
 InstantiationDependent)) {
   Diag(TemplateNameLoc, diag::err_partial_spec_fully_specialized)
   << VarTemplate->getDeclName();
-  IsPartialSpecialization = false;
+  // FIXME: Can this ever get triggered?  If so, we need test.  If not,
+  // should this if be removed?
+  return true;
 }
 
 if (isSameAsPrimaryTemplate(VarTemplate->getTemplateParameters(),
@@ -7276,7 +7278,7 @@
 TemplateArgs.arguments(), InstantiationDependent)) {
   Diag(TemplateNameLoc, diag::err_partial_spec_fully_specialized)
 << ClassTemplate->getDeclName();
-  isPartialSpecialization = false;
+  return true;
 }
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34179: [SEMA] PR32318 Handle -ast-dump-all properly.

[don hinton via Phabricator via cfe-commits]

hintonda created this revision.

Handle -ast-dump-all when passed as the only option.


https://reviews.llvm.org/D34179

Files:
  lib/Frontend/ASTConsumers.cpp
  test/Coverage/ast-printing.c
  test/Coverage/ast-printing.cpp


Index: test/Coverage/ast-printing.cpp
===
--- test/Coverage/ast-printing.cpp
+++ test/Coverage/ast-printing.cpp
@@ -3,6 +3,7 @@
 // RUN: %clang_cc1 -std=c++14 -ast-print %t.1.cpp -o %t.2.cpp
 // RUN: diff %t.1.cpp %t.2.cpp
 // RUN: %clang_cc1 -std=c++14 -ast-dump %s
+// RUN: %clang_cc1 -std=c++14 -ast-dump-all %s
 // RUN: %clang_cc1 -std=c++14 -print-decl-contexts %s
 // RUN: %clang_cc1 -std=c++14 -fdump-record-layouts %s
 
Index: test/Coverage/ast-printing.c
===
--- test/Coverage/ast-printing.c
+++ test/Coverage/ast-printing.c
@@ -3,6 +3,7 @@
 // RUN: %clang_cc1 -ast-print %t.1.c -o %t.2.c
 // RUN: diff %t.1.c %t.2.c
 // RUN: %clang_cc1 -ast-dump %s
+// RUN: %clang_cc1 -ast-dump-all %s
 // RUN: %clang_cc1 -print-decl-contexts %s
 
 #include "c-language-features.inc"
Index: lib/Frontend/ASTConsumers.cpp
===
--- lib/Frontend/ASTConsumers.cpp
+++ lib/Frontend/ASTConsumers.cpp
@@ -142,7 +142,7 @@
 bool DumpDecls,
 bool Deserialize,
 bool DumpLookups) {
-  assert((DumpDecls || DumpLookups) && "nothing to dump");
+  assert((DumpDecls || Deserialize || DumpLookups) && "nothing to dump");
   return llvm::make_unique(nullptr,
Deserialize ? ASTPrinter::DumpFull :
DumpDecls ? ASTPrinter::Dump :


Index: test/Coverage/ast-printing.cpp
===
--- test/Coverage/ast-printing.cpp
+++ test/Coverage/ast-printing.cpp
@@ -3,6 +3,7 @@
 // RUN: %clang_cc1 -std=c++14 -ast-print %t.1.cpp -o %t.2.cpp
 // RUN: diff %t.1.cpp %t.2.cpp
 // RUN: %clang_cc1 -std=c++14 -ast-dump %s
+// RUN: %clang_cc1 -std=c++14 -ast-dump-all %s
 // RUN: %clang_cc1 -std=c++14 -print-decl-contexts %s
 // RUN: %clang_cc1 -std=c++14 -fdump-record-layouts %s
 
Index: test/Coverage/ast-printing.c
===
--- test/Coverage/ast-printing.c
+++ test/Coverage/ast-printing.c
@@ -3,6 +3,7 @@
 // RUN: %clang_cc1 -ast-print %t.1.c -o %t.2.c
 // RUN: diff %t.1.c %t.2.c
 // RUN: %clang_cc1 -ast-dump %s
+// RUN: %clang_cc1 -ast-dump-all %s
 // RUN: %clang_cc1 -print-decl-contexts %s
 
 #include "c-language-features.inc"
Index: lib/Frontend/ASTConsumers.cpp
===
--- lib/Frontend/ASTConsumers.cpp
+++ lib/Frontend/ASTConsumers.cpp
@@ -142,7 +142,7 @@
 bool DumpDecls,
 bool Deserialize,
 bool DumpLookups) {
-  assert((DumpDecls || DumpLookups) && "nothing to dump");
+  assert((DumpDecls || Deserialize || DumpLookups) && "nothing to dump");
   return llvm::make_unique(nullptr,
Deserialize ? ASTPrinter::DumpFull :
DumpDecls ? ASTPrinter::Dump :
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34179: [Frontend] PR32318 Handle -ast-dump-all properly.

[don hinton via Phabricator via cfe-commits]

hintonda added a comment.

Great, thanks Aaron.

Could you commit it for me when you get a chance?


https://reviews.llvm.org/D34179



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

[PATCH] D34179: [Frontend] PR32318 Handle -ast-dump-all properly.

[don hinton via Phabricator via cfe-commits]

hintonda added a comment.

I have, but just haven't heard back yet...  Thanks again...


https://reviews.llvm.org/D34179



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

[PATCH] D16971: [Sema] PR26077 Fixed crash when partial specialization is missing required parameters

[don hinton via Phabricator via cfe-commits]

hintonda added a comment.

ping...


https://reviews.llvm.org/D16971



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

[PATCH] D34383: [Sema] PR32953: Fix hard crash of implicit instantiation of undefined template preceeded by scoped variable

[don hinton via Phabricator via cfe-commits]

hintonda created this revision.

Reset ScopeSpec when parsing class, struct, etc..  Fixes PR32953.


https://reviews.llvm.org/D34383

Files:
  lib/Parse/ParseDecl.cpp
  test/SemaTemplate/instantiate-complete.cpp


Index: test/SemaTemplate/instantiate-complete.cpp
===
--- test/SemaTemplate/instantiate-complete.cpp
+++ test/SemaTemplate/instantiate-complete.cpp
@@ -134,6 +134,14 @@
   template class B; // expected-note {{in instantiation}}
 }
 
+namespace PR32953 {
+  struct bar {};
+  template  struct Temp; // expected-note {{template is declared 
here}}
+  PR32953::bar struct Temp::foo(); // expected-error {{implicit 
instantiation of undefined template 'PR32953::Temp'}} \
+ expected-error {{cannot combine with 
previous 'type-name' declaration specifier}} \
+ expected-error {{expected 
unqualified-id}}
+}
+
 namespace PR8425 {
   template 
   class BaseT {};
Index: lib/Parse/ParseDecl.cpp
===
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/ParseDecl.cpp
@@ -3581,6 +3581,8 @@
   tok::TokenKind Kind = Tok.getKind();
   ConsumeToken();
 
+  DS.getTypeSpecScope() = CXXScopeSpec();
+
   // These are attributes following class specifiers.
   // To produce better diagnostic, we parse them when
   // parsing class specifier.


Index: test/SemaTemplate/instantiate-complete.cpp
===
--- test/SemaTemplate/instantiate-complete.cpp
+++ test/SemaTemplate/instantiate-complete.cpp
@@ -134,6 +134,14 @@
   template class B; // expected-note {{in instantiation}}
 }
 
+namespace PR32953 {
+  struct bar {};
+  template  struct Temp; // expected-note {{template is declared here}}
+  PR32953::bar struct Temp::foo(); // expected-error {{implicit instantiation of undefined template 'PR32953::Temp'}} \
+ expected-error {{cannot combine with previous 'type-name' declaration specifier}} \
+ expected-error {{expected unqualified-id}}
+}
+
 namespace PR8425 {
   template 
   class BaseT {};
Index: lib/Parse/ParseDecl.cpp
===
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/ParseDecl.cpp
@@ -3581,6 +3581,8 @@
   tok::TokenKind Kind = Tok.getKind();
   ConsumeToken();
 
+  DS.getTypeSpecScope() = CXXScopeSpec();
+
   // These are attributes following class specifiers.
   // To produce better diagnostic, we parse them when
   // parsing class specifier.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41248: [libcxx] Suppress unused warning on apple.

[Don Hinton via Phabricator via cfe-commits]

hintonda created this revision.
hintonda added a reviewer: EricWF.

This warning is already suppressed on non-apple platforms, so
this change just suppresses it on apple as well.


Repository:
  rCXX libc++

https://reviews.llvm.org/D41248

Files:
  src/experimental/filesystem/operations.cpp


Index: src/experimental/filesystem/operations.cpp
===
--- src/experimental/filesystem/operations.cpp
+++ src/experimental/filesystem/operations.cpp
@@ -512,6 +512,7 @@
 
 #if defined(__APPLE__)
 TimeSpec extract_mtime(StatT const& st) { return st.st_mtimespec; }
+__attribute__((unused)) // Suppress warning
 TimeSpec extract_atime(StatT const& st) { return st.st_atimespec; }
 #else
 TimeSpec extract_mtime(StatT const& st) { return st.st_mtim; }


Index: src/experimental/filesystem/operations.cpp
===
--- src/experimental/filesystem/operations.cpp
+++ src/experimental/filesystem/operations.cpp
@@ -512,6 +512,7 @@
 
 #if defined(__APPLE__)
 TimeSpec extract_mtime(StatT const& st) { return st.st_mtimespec; }
+__attribute__((unused)) // Suppress warning
 TimeSpec extract_atime(StatT const& st) { return st.st_atimespec; }
 #else
 TimeSpec extract_mtime(StatT const& st) { return st.st_mtim; }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41259: [debuginfo] Remove temporary FIXME.

[Don Hinton via Phabricator via cfe-commits]

hintonda created this revision.
hintonda added reviewers: zturner, aprantl.
Herald added a subscriber: JDevlieghere.

Now that r320495, "[debuginfo-tests] Support moving
debuginfo-tests to llvm/projects," has landed, remove temporary FIXME
that supported the old mechanism.


Repository:
  rC Clang

https://reviews.llvm.org/D41259

Files:
  test/lit.cfg.py


Index: test/lit.cfg.py
===
--- test/lit.cfg.py
+++ test/lit.cfg.py
@@ -31,7 +31,7 @@
 # excludes: A list of directories to exclude from the testsuite. The 'Inputs'
 # subdirectories contain auxiliary inputs for various tests in their parent
 # directories.
-config.excludes = ['Inputs', 'CMakeLists.txt', 'README.txt', 'LICENSE.txt']
+config.excludes = ['Inputs', 'CMakeLists.txt', 'README.txt', 'LICENSE.txt', 
'debuginfo-tests']
 
 # test_source_root: The root path where tests are located.
 config.test_source_root = os.path.dirname(__file__)
@@ -62,16 +62,6 @@
 'clang-func-mapping'), unresolved='ignore'),
 ]
 
-# FIXME: This logic can be removed once all buildbots have moved
-# debuginfo-test from clang/test to llvm/projects or monorepo.
-if os.path.exists(os.path.join(config.test_source_root, 'debuginfo-tests')):
-  if os.path.isfile(
-  os.path.join(config.test_source_root, 'debuginfo-tests', 'lit.cfg.py')):
-config.excludes.append('debuginfo-tests')
-  else:
-tools.append(ToolSubst('%test_debuginfo', command=os.path.join(
-  config.llvm_src_root, 'utils', 'test_debuginfo.pl')))
-
 if config.clang_examples:
 tools.append('clang-interpreter')
 


Index: test/lit.cfg.py
===
--- test/lit.cfg.py
+++ test/lit.cfg.py
@@ -31,7 +31,7 @@
 # excludes: A list of directories to exclude from the testsuite. The 'Inputs'
 # subdirectories contain auxiliary inputs for various tests in their parent
 # directories.
-config.excludes = ['Inputs', 'CMakeLists.txt', 'README.txt', 'LICENSE.txt']
+config.excludes = ['Inputs', 'CMakeLists.txt', 'README.txt', 'LICENSE.txt', 'debuginfo-tests']
 
 # test_source_root: The root path where tests are located.
 config.test_source_root = os.path.dirname(__file__)
@@ -62,16 +62,6 @@
 'clang-func-mapping'), unresolved='ignore'),
 ]
 
-# FIXME: This logic can be removed once all buildbots have moved
-# debuginfo-test from clang/test to llvm/projects or monorepo.
-if os.path.exists(os.path.join(config.test_source_root, 'debuginfo-tests')):
-  if os.path.isfile(
-  os.path.join(config.test_source_root, 'debuginfo-tests', 'lit.cfg.py')):
-config.excludes.append('debuginfo-tests')
-  else:
-tools.append(ToolSubst('%test_debuginfo', command=os.path.join(
-  config.llvm_src_root, 'utils', 'test_debuginfo.pl')))
-
 if config.clang_examples:
 tools.append('clang-interpreter')
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Don Hinton via Phabricator via cfe-commits]

hintonda added inline comments.



Comment at: include/clang/Config/config.h.cmake:40
+#cmakedefine CLANG_CONFIG_FILE_SYSTEM_DIR ${CLANG_CONFIG_FILE_SYSTEM_DIR}
+#cmakedefine CLANG_CONFIG_FILE_USER_DIR ${CLANG_CONFIG_FILE_USER_DIR}
+

These need to be in quotes since you assign them to a std::string, i.e.:

```
#cmakedefine CLANG_CONFIG_FILE_SYSTEM_DIR "${CLANG_CONFIG_FILE_SYSTEM_DIR}"
#cmakedefine CLANG_CONFIG_FILE_USER_DIR "${CLANG_CONFIG_FILE_USER_DIR}"
```


https://reviews.llvm.org/D24933



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

[PATCH] D41248: [libcxx] Suppress unused warning on apple.

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

ping...


Repository:
  rCXX libc++

https://reviews.llvm.org/D41248



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

[PATCH] D41621: [cmake] Fix path problems when cross compiling.

[Don Hinton via Phabricator via cfe-commits]

hintonda created this revision.
hintonda added reviewers: EricWF, compnerd, phosek, beanz.
Herald added a subscriber: mgorny.

When cross compiling, users can set CMAKE_FIND_ROOT_PATH to
the sysroot of the TARGET system which adds the sysroot prefix to the
paths used in the FIND_XXX commands.  Since these FIND_PATH commands
use absolute paths found on the HOST system, pass
NO_CMAKE_FIND_ROOT_PATH to keep the prefix from being added.


https://reviews.llvm.org/D41621

Files:
  CMakeLists.txt


Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -329,7 +329,8 @@
   PATHS ${LLVM_MAIN_SRC_DIR}/projects/libcxx/include
 ${LLVM_MAIN_SRC_DIR}/runtimes/libcxx/include
   NO_DEFAULT_PATH
-)
+  NO_CMAKE_FIND_ROOT_PATH
+  )
 if ((NOT LIBUNWIND_STANDALONE_BUILD OR HAVE_LIBCXX) AND
 IS_DIRECTORY "${LIBUNWIND_LIBCXX_INCLUDES_INTERNAL}")
   set(LIBUNWIND_CXX_INCLUDE_PATHS_DEFAULT 
"${LIBUNWIND_LIBCXX_INCLUDES_INTERNAL}")


Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -329,7 +329,8 @@
   PATHS ${LLVM_MAIN_SRC_DIR}/projects/libcxx/include
 ${LLVM_MAIN_SRC_DIR}/runtimes/libcxx/include
   NO_DEFAULT_PATH
-)
+  NO_CMAKE_FIND_ROOT_PATH
+  )
 if ((NOT LIBUNWIND_STANDALONE_BUILD OR HAVE_LIBCXX) AND
 IS_DIRECTORY "${LIBUNWIND_LIBCXX_INCLUDES_INTERNAL}")
   set(LIBUNWIND_CXX_INCLUDE_PATHS_DEFAULT "${LIBUNWIND_LIBCXX_INCLUDES_INTERNAL}")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41622: [cmake] Fix path and flag problems when cross compiling.

[Don Hinton via Phabricator via cfe-commits]

hintonda created this revision.
hintonda added reviewers: EricWF, compnerd, phosek, vkalintiris, beanz.
Herald added a subscriber: mgorny.

When cross compiling, users can set CMAKE_FIND_ROOT_PATH to
the sysroot of the TARGET system which adds the sysroot prefix to the
paths used in the FIND_XXX commands.  Since these FIND_PATH commands
use absolute paths found on the HOST system, pass
NO_CMAKE_FIND_ROOT_PATH to keep the prefix from being added.

Don't overwrite CMAKE_REQUIRED_FLAGS when adding temporary flags.


Repository:
  rCXX libc++

https://reviews.llvm.org/D41622

Files:
  CMakeLists.txt
  cmake/Modules/CheckLibcxxAtomic.cmake


Index: cmake/Modules/CheckLibcxxAtomic.cmake
===
--- cmake/Modules/CheckLibcxxAtomic.cmake
+++ cmake/Modules/CheckLibcxxAtomic.cmake
@@ -9,7 +9,7 @@
 
 function(check_cxx_atomics varname)
   set(OLD_CMAKE_REQUIRED_FLAGS ${CMAKE_REQUIRED_FLAGS})
-  set(CMAKE_REQUIRED_FLAGS "-nodefaultlibs -std=c++11 -nostdinc++ -isystem 
${LIBCXX_SOURCE_DIR}/include")
+  set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -nodefaultlibs -std=c++11 
-nostdinc++ -isystem ${LIBCXX_SOURCE_DIR}/include")
   if (${LIBCXX_GCC_TOOLCHAIN})
 set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} 
--gcc-toolchain=${LIBCXX_GCC_TOOLCHAIN}")
   endif()
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -123,6 +123,7 @@
   ${LLVM_MAIN_SRC_DIR}/runtimes/libcxxabi/include
   ${LLVM_MAIN_SRC_DIR}/../libcxxabi/include
 NO_DEFAULT_PATH
+NO_CMAKE_FIND_ROOT_PATH
   )
   if (LIBCXX_TARGETING_MSVC)
 # FIXME: Figure out how to configure the ABI library on Windows.


Index: cmake/Modules/CheckLibcxxAtomic.cmake
===
--- cmake/Modules/CheckLibcxxAtomic.cmake
+++ cmake/Modules/CheckLibcxxAtomic.cmake
@@ -9,7 +9,7 @@
 
 function(check_cxx_atomics varname)
   set(OLD_CMAKE_REQUIRED_FLAGS ${CMAKE_REQUIRED_FLAGS})
-  set(CMAKE_REQUIRED_FLAGS "-nodefaultlibs -std=c++11 -nostdinc++ -isystem ${LIBCXX_SOURCE_DIR}/include")
+  set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -nodefaultlibs -std=c++11 -nostdinc++ -isystem ${LIBCXX_SOURCE_DIR}/include")
   if (${LIBCXX_GCC_TOOLCHAIN})
 set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} --gcc-toolchain=${LIBCXX_GCC_TOOLCHAIN}")
   endif()
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -123,6 +123,7 @@
   ${LLVM_MAIN_SRC_DIR}/runtimes/libcxxabi/include
   ${LLVM_MAIN_SRC_DIR}/../libcxxabi/include
 NO_DEFAULT_PATH
+NO_CMAKE_FIND_ROOT_PATH
   )
   if (LIBCXX_TARGETING_MSVC)
 # FIXME: Figure out how to configure the ABI library on Windows.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41623: [cmake] Fix path problems when cross compiling.

[Don Hinton via Phabricator via cfe-commits]

hintonda created this revision.
hintonda added reviewers: EricWF, compnerd, phosek, beanz.
Herald added a subscriber: mgorny.

When cross compiling, users can set CMAKE_FIND_ROOT_PATH to
the sysroot of the TARGET system which adds the sysroot prefix to the
paths used in the FIND_XXX commands.  Since these FIND_PATH commands
use absolute paths found on the HOST system, pass
NO_CMAKE_FIND_ROOT_PATH to keep the prefix from being added.


Repository:
  rCXXA libc++abi

https://reviews.llvm.org/D41623

Files:
  CMakeLists.txt


Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -115,6 +115,7 @@
 ${CMAKE_BINARY_DIR}/${LIBCXXABI_LIBCXX_INCLUDES}
 ${LIBCXXABI_LIBCXX_INCLUDE_DIRS}
 ${LLVM_INCLUDE_DIR}/c++/v1
+  NO_CMAKE_FIND_ROOT_PATH
   )
 
 set(LIBCXXABI_LIBCXX_INCLUDES "${LIBCXXABI_LIBCXX_INCLUDES}" CACHE PATH
@@ -127,6 +128,7 @@
 ${LIBCXXABI_LIBCXX_INCLUDES}/../
 ${LIBCXXABI_LIBCXX_SRC_DIRS}
   NO_DEFAULT_PATH
+  NO_CMAKE_FIND_ROOT_PATH
   )
 
 if (LIBCXXABI_LIBCXX_PATH STREQUAL "LIBCXXABI_LIBCXX_PATH-NOTFOUND")
@@ -425,6 +427,7 @@
   ${LLVM_MAIN_SRC_DIR}/projects/libunwind/include
   ${LLVM_MAIN_SRC_DIR}/runtimes/libunwind/include
 NO_DEFAULT_PATH
+NO_CMAKE_FIND_ROOT_PATH
   )
 
   if (LIBCXXABI_LIBUNWIND_INCLUDES_INTERNAL STREQUAL 
"LIBCXXABI_LIBUNWIND_INCLUDES_INTERNAL-NOTFOUND")


Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -115,6 +115,7 @@
 ${CMAKE_BINARY_DIR}/${LIBCXXABI_LIBCXX_INCLUDES}
 ${LIBCXXABI_LIBCXX_INCLUDE_DIRS}
 ${LLVM_INCLUDE_DIR}/c++/v1
+  NO_CMAKE_FIND_ROOT_PATH
   )
 
 set(LIBCXXABI_LIBCXX_INCLUDES "${LIBCXXABI_LIBCXX_INCLUDES}" CACHE PATH
@@ -127,6 +128,7 @@
 ${LIBCXXABI_LIBCXX_INCLUDES}/../
 ${LIBCXXABI_LIBCXX_SRC_DIRS}
   NO_DEFAULT_PATH
+  NO_CMAKE_FIND_ROOT_PATH
   )
 
 if (LIBCXXABI_LIBCXX_PATH STREQUAL "LIBCXXABI_LIBCXX_PATH-NOTFOUND")
@@ -425,6 +427,7 @@
   ${LLVM_MAIN_SRC_DIR}/projects/libunwind/include
   ${LLVM_MAIN_SRC_DIR}/runtimes/libunwind/include
 NO_DEFAULT_PATH
+NO_CMAKE_FIND_ROOT_PATH
   )
 
   if (LIBCXXABI_LIBUNWIND_INCLUDES_INTERNAL STREQUAL "LIBCXXABI_LIBUNWIND_INCLUDES_INTERNAL-NOTFOUND")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41623: [cmake] [libcxxabi] Fix path problems when cross compiling.

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In https://reviews.llvm.org/D41623#965271, @compnerd wrote:

> I think that it might be better to handle this as a single global change:
>
>   if(CMAKE_FIND_ROOT_PATH)
> set(CMAKE_FIND_ROOT_PATH_MODE_INCLUDE NEVER)
>   endif()


When I cross compile on Darwin and target Linux, I set sysroot and only want 
look for headers and libs in the new sysroot, not the host system.  However, in 
this case, I want to look for the just built headers.  This change helps 
support this.

As for global settings, this is what I'm setting in my toolchain file:

  SET(CMAKE_FIND_ROOT_PATH "${sysroot}" CACHE STRING "" FORCE)
  # adjust the default behavior of the FIND_XXX() commands:
  # search headers and libraries in the target environment, search
  # programs in the host environment
  set(CMAKE_FIND_ROOT_PATH_MODE_PROGRAM NEVER CACHE STRING "" FORCE)
  set(CMAKE_FIND_ROOT_PATH_MODE_LIBRARY ONLY CACHE STRING "" FORCE)
  set(CMAKE_FIND_ROOT_PATH_MODE_INCLUDE ONLY CACHE STRING "" FORCE)


Repository:
  rCXXA libc++abi

https://reviews.llvm.org/D41623



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

[PATCH] D41622: [cmake] [libcxx] Fix path and flag problems when cross compiling.

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In https://reviews.llvm.org/D41622#965280, @phosek wrote:

> Would it be possible to split the `CMAKE_REQUIRED_FLAGS` fix into a separate 
> change since it's unrelated to the `CMAKE_FIND_ROOT_PATH` bit?


Sure, but both are needed to actually work.  This fixes cross compiling across 
systems, e.g., from Darwin to Linux.  Without both, configuration will fail.

Please note, cross system compiling also requires a pending patch to cmake:  
https://gitlab.kitware.com/cmake/cmake/merge_requests/1620


Repository:
  rCXX libc++

https://reviews.llvm.org/D41622



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

[PATCH] D41622: [cmake] [libcxx] Fix path and flag problems when cross compiling.

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

Btw, here's the WIP toolchain file I"m using, which might help clear up why 
these changes is necessary:

  https://github.com/donhinton/misc/blob/master/cmake/linux-toolchain.cmake


Repository:
  rCXX libc++

https://reviews.llvm.org/D41622



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

[PATCH] D41660: [cmake] Add new linux toolchain file

[Don Hinton via Phabricator via cfe-commits]

hintonda created this revision.
hintonda added reviewers: compnerd, beanz, phosek.
Herald added a subscriber: mgorny.

Add new linux toolchain file that allows cross compiling to
linux from other systems, e.g., Darwin.

Also, add a new variable, ADDITIONAL_CLANG_BOOTSTRAP_DEPS, which
allows adding additional dependencies to clang-bootstrap-deps.


Repository:
  rC Clang

https://reviews.llvm.org/D41660

Files:
  CMakeLists.txt
  cmake/caches/linux-toolchain.cmake

Index: cmake/caches/linux-toolchain.cmake
===
--- /dev/null
+++ cmake/caches/linux-toolchain.cmake
@@ -0,0 +1,142 @@
+# This file is intended to support cross compiling a linux toolchain
+# on any host system, includind Darwin.
+#
+# Usage: cmake -GNinja -DSYSROOT= [OPTIONS] -C ../clang/cmake/caches/linux-toolchain.cmake  ../llvm
+#
+#   OPTIONS:
+# Regular options apply to stage1.
+# BOOTSTRAP_ options apply to stage2, and should use the BOOTSTRAP_ prefix.
+#
+# Then run "ninja stage2" to cross compile.  Bins and libs can be
+# found in tools/clang/stage2-bins.
+#
+# Known issues:
+#
+#  1) This toolchain assumes a flat, mono-repo layout.
+#
+#  2) Several sub-projects, including libcxx, libcxxabi, and
+# libunwind, use FIND_PATH() to find the source path for headers
+# of other sub-projects on the host system, but fail because they
+# don't specify NO_CMAKE_FIND_ROOT_PATH.  The following pathes are
+# reqired to build these projects:
+#
+#   libcxx   https://reviews.llvm.org/D41622
+#   libcxxabihttps://reviews.llvm.org/D41623
+#   libunwindhttps://reviews.llvm.org/D41621
+#
+#  3) Libraries in the compiler-rt sub-project fail to link with the
+# following error:
+#
+#   bin/ld.lld: error:
+#   projects/compiler-rt/lib/sanitizer_common/CMakeFiles/RTSanitizerCommon.x86_64.dir/sanitizer_linux_x86_64.S.o:
+#   invalid data encoding
+#
+#  4) FIND_PACKAGE can fail if the package file uses PkgConfig, e.g.,
+# FindLibXml2.cmake, since the pkg-config program runs on the
+# host, and the variables set by PKG_CHECK_MODULES reference the
+# host system, not the target.
+#
+#  5) Stage2 configuration fails for several sub-projects, including
+# libcxx, libcxxabi, and libunwind, with the following error:
+#
+#   CMake Error at .../llvm_project/libunwind/src/CMakeLists.txt:110 (add_library):
+#   The install of the unwind_shared target requires changing an
+#   RPATH from the build tree, but this is not supported with the
+#   Ninja generator unless on an ELF-based platform.  The
+#   CMAKE_BUILD_WITH_INSTALL_RPATH variable may be set to avoid
+#   this relinking step.
+#
+
+if(NOT DEFINED SYSROOT AND NOT DEFINED CMAKE_SYSROOT)
+  message(FATAL_ERROR "Missing required option -DSYSROOT=.")
+endif()
+
+if(DEFINED SYSROOT)
+  # Since we just want to build a bootstrap compiler, turn off as much
+  # as possible.
+  set(LLVM_INCLUDE_DOCS OFF CACHE BOOL "")
+  set(LLVM_INCLUDE_EXAMPLES OFF CACHE BOOL "")
+  set(LLVM_INCLUDE_RUNTIMES OFF CACHE BOOL "")
+  set(LLVM_INCLUDE_TESTS OFF CACHE BOOL "")
+  set(LLVM_INCLUDE_UTILS OFF CACHE BOOL "")
+  set(CLANG_BUILD_TOOLS OFF CACHE BOOL "")
+  set(CLANG_ENABLE_ARCMT OFF CACHE BOOL "")
+  set(CLANG_ENABLE_STATIC_ANALYZER OFF CACHE BOOL "")
+
+  set(LLVM_TARGETS_TO_BUILD Native CACHE STRING "")
+  set(CMAKE_BUILD_TYPE RELEASE CACHE STRING "")
+  set(LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
+
+  # Make sure at least clang and lld are included.
+  list(APPEND LLVM_ENABLE_PROJECTS clang lld)
+  set(LLVM_ENABLE_PROJECTS ${LLVM_ENABLE_PROJECTS} CACHE STRING "")
+
+  # Passing -fuse-ld=lld is hard for cmake to handle correctly, so
+  # make lld the default linker.
+  set(CLANG_DEFAULT_LINKER lld CACHE STRING "" FORCE)
+
+  # Since LLVM_ENABLE_PROJECTS gets passed automatically to the next
+  # stage, use another variable to pass the desired projects to stage2.
+  if(DEFINED BOOTSTRAP_LLVM_ENABLE_PROJECTS)
+set(BOOTSTRAP_STAGE2_PROJECTS ${BOOTSTRAP_LLVM_ENABLE_PROJECTS} CACHE STRING "" FORCE)
+unset(BOOTSTRAP_LLVM_ENABLE_PROJECTS CACHE)
+  else()
+set(BOOTSTRAP_STAGE2_PROJECTS "clang;libcxx;libcxxabi;libunwind" CACHE STRING "" FORCE)
+  endif()
+
+  # Required on non-elf hosts.
+  set(ADDITIONAL_CLANG_BOOTSTRAP_DEPS "lld;llvm-ar;llvm-ranlib" CACHE STRING "")
+
+  set(CLANG_ENABLE_BOOTSTRAP ON CACHE BOOL "")
+  set(CLANG_BOOTSTRAP_CMAKE_ARGS
+-DCMAKE_SYSROOT=${SYSROOT}
+-DCMAKE_TOOLCHAIN_FILE=${CMAKE_CURRENT_LIST_FILE} CACHE STRING "")
+else()
+  set(CMAKE_SYSTEM_NAME Linux CACHE STRING "" FORCE)
+  set(LLVM_DEFAULT_TARGET_TRIPLE "x86_64-unknown-linux-gnu" CACHE STRING "" FORCE)
+
+  # Set default, but allow overries.
+  set(CMAKE_BUILD_TYPE Release CACHE STRING "")
+
+  # Always use STAGE2_PROJECTS Use FORCE to override passed vaiable.
+  set(LLVM_ENABLE_PROJECTS ${STAGE2_PROJECTS} CACHE STRING "" FORCE)
+
+  # Use bootstrap tools.
+  get_filena

[PATCH] D41660: [cmake] Add new linux toolchain file

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In https://reviews.llvm.org/D41660#965656, @smeenai wrote:

> Why is this a cache file rather than a toolchain file (but passing itself as 
> a toolchain file to CMake under some circumstances?) Aren't toolchain files 
> traditionally used for cross-compilation?


Thanks for taking a look.

Yes, this is for cross-compiling clang+llvm for Linux on Darwin -- and possibly 
Windows to Linux, but that hasn't been tested -- or Linux to Linux if you have 
completely different system files.  It enforces using --sysroot to find the 
targets headers and libraries.

Cache files are preferred since they are only loaded once, but toolchain files 
are more flexible -- particularly when setting -target and --sysroot.  Users 
shouldn't care if the cache file reloads itself as a toolchain file, and 
keeping everything is one file makes it easier to understand.  This version 
doesn't include arch-specific builtins and runtimes, but that could easily be 
added.

Also, I'm happy to rename it if that would help.


Repository:
  rC Clang

https://reviews.llvm.org/D41660



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

[PATCH] D41660: [cmake] Add new linux toolchain file

[Don Hinton via Phabricator via cfe-commits]

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

Use CMAKE_(C|CXX)_COMPILER_TARGET instead of
CMAKE_(C|CXX)_COMPILER_ARG1, and pass all target variables via
CLANG_BOOTSTRAP_CMAKE_ARGS.

Add variable tests and Fix/update comments.


Repository:
  rC Clang

https://reviews.llvm.org/D41660

Files:
  CMakeLists.txt
  cmake/caches/linux-toolchain.cmake

Index: cmake/caches/linux-toolchain.cmake
===
--- /dev/null
+++ cmake/caches/linux-toolchain.cmake
@@ -0,0 +1,144 @@
+# This file is intended to support cross compiling a linux toolchain
+# on any host system, includind Darwin.
+#
+# Usage: cmake -GNinja -DSYSROOT= [OPTIONS] -C ../clang/cmake/caches/linux-toolchain.cmake  ../llvm
+#
+#   OPTIONS:
+# Regular options apply to stage1.
+# BOOTSTRAP_ options apply to stage2, and should use the BOOTSTRAP_ prefix.
+#
+# Then run "ninja stage2" to cross compile.  Bins and libs can be
+# found in tools/clang/stage2-bins.
+#
+# Known issues:
+#
+#  1) This toolchain assumes a flat, mono-repo layout.
+#
+#  2) Several sub-projects, including libcxx, libcxxabi, and
+# libunwind, use FIND_PATH() to find the source path for headers
+# of other sub-projects on the host system, but fail because they
+# don't specify NO_CMAKE_FIND_ROOT_PATH.  The following patches are
+# reqired to build these projects:
+#
+#   libcxx   https://reviews.llvm.org/D41622
+#   libcxxabihttps://reviews.llvm.org/D41623
+#   libunwindhttps://reviews.llvm.org/D41621
+#
+#  3) Libraries in the compiler-rt sub-project fail to link with the
+# following error:
+#
+#   bin/ld.lld: error:
+#   projects/compiler-rt/lib/sanitizer_common/CMakeFiles/RTSanitizerCommon.x86_64.dir/sanitizer_linux_x86_64.S.o:
+#   invalid data encoding
+#
+#  4) FIND_PACKAGE can fail if the package file uses PkgConfig, e.g.,
+# FindLibXml2.cmake, since the pkg-config program runs on the
+# host, and the variables set by PKG_CHECK_MODULES reference the
+# host system, not the target.
+#
+#  5) Stage2 configuration fails for several sub-projects, including
+# libcxx, libcxxabi, and libunwind, with the following error:
+#
+#   CMake Error at .../llvm_project/libunwind/src/CMakeLists.txt:110 (add_library):
+#   The install of the unwind_shared target requires changing an
+#   RPATH from the build tree, but this is not supported with the
+#   Ninja generator unless on an ELF-based platform.  The
+#   CMAKE_BUILD_WITH_INSTALL_RPATH variable may be set to avoid
+#   this relinking step.
+#
+
+if(NOT DEFINED SYSROOT AND NOT DEFINED CMAKE_SYSROOT)
+  message(FATAL_ERROR "Missing required option -DSYSROOT=.")
+endif()
+
+if(DEFINED SYSROOT)
+  # Since we just want to build a bootstrap compiler, turn off as much
+  # as possible.
+  set(LLVM_INCLUDE_DOCS OFF CACHE BOOL "")
+  set(LLVM_INCLUDE_EXAMPLES OFF CACHE BOOL "")
+  set(LLVM_INCLUDE_RUNTIMES OFF CACHE BOOL "")
+  set(LLVM_INCLUDE_TESTS OFF CACHE BOOL "")
+  set(LLVM_INCLUDE_UTILS OFF CACHE BOOL "")
+  set(CLANG_BUILD_TOOLS OFF CACHE BOOL "")
+  set(CLANG_ENABLE_ARCMT OFF CACHE BOOL "")
+  set(CLANG_ENABLE_STATIC_ANALYZER OFF CACHE BOOL "")
+
+  set(LLVM_TARGETS_TO_BUILD Native CACHE STRING "")
+  set(CMAKE_BUILD_TYPE RELEASE CACHE STRING "")
+  set(LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
+
+  # Make sure at least clang and lld are included.
+  list(APPEND LLVM_ENABLE_PROJECTS clang lld)
+  set(LLVM_ENABLE_PROJECTS ${LLVM_ENABLE_PROJECTS} CACHE STRING "")
+
+  # Passing -fuse-ld=lld is hard for cmake to handle correctly, so
+  # make lld the default linker.
+  set(CLANG_DEFAULT_LINKER lld CACHE STRING "" FORCE)
+
+  # Since LLVM_ENABLE_PROJECTS gets passed automatically to the next
+  # stage, use another variable to pass the desired projects to stage2.
+  if(DEFINED BOOTSTRAP_LLVM_ENABLE_PROJECTS)
+set(BOOTSTRAP_STAGE2_PROJECTS ${BOOTSTRAP_LLVM_ENABLE_PROJECTS} CACHE STRING "" FORCE)
+unset(BOOTSTRAP_LLVM_ENABLE_PROJECTS CACHE)
+  else()
+set(BOOTSTRAP_STAGE2_PROJECTS "clang;libcxx;libcxxabi;libunwind" CACHE STRING "" FORCE)
+  endif()
+
+  # Required on non-elf hosts.
+  set(ADDITIONAL_CLANG_BOOTSTRAP_DEPS "lld;llvm-ar;llvm-ranlib" CACHE STRING "")
+
+  if(NOT DEFINED TRIPLE)
+set(TRIPLE "x86_64-unknown-linux-gnu")
+  endif()
+
+  set(CLANG_ENABLE_BOOTSTRAP ON CACHE BOOL "")
+  set(CLANG_BOOTSTRAP_CMAKE_ARGS
+-DCMAKE_SYSROOT=${SYSROOT}
+-DLLVM_DEFAULT_TARGET_TRIPLE=${TRIPLE}
+-DCMAKE_C_COMPILER_TARGET=${TRIPLE}
+-DCMAKE_CXX_COMPILER_TARGET=${TRIPLE}
+-DCMAKE_TOOLCHAIN_FILE=${CMAKE_CURRENT_LIST_FILE} CACHE STRING "")
+else()
+  set(CMAKE_SYSTEM_NAME Linux CACHE STRING "" FORCE)
+
+  # Set default, but allow overries.
+  if(NOT DEFINED CMAKE_BUILD_TYPE)
+set(CMAKE_BUILD_TYPE Release CACHE STRING "")
+  endif()
+
+  # Always use STAGE2_PROJECTS Use FORCE to override passed vaiable.
+  set(LLVM_ENABLE_PROJECTS

[PATCH] D41660: [cmake] Add new linux toolchain file

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In https://reviews.llvm.org/D41660#965877, @beanz wrote:

> You should split the CMake cache file you created into two files, (1) a CMake 
> Cache to manage the build configuration and (2) a tool chain file for 
> targeting Linux. As @semeenai pointed out we absolutly want the behavior of 
> the toolchain file being loaded multiple times. That is the correct way this 
> build should work.


I really like keeping this in a single file, but will break it up if necessary.

The `if` part of the if/else is used in stage1 as a cache file, and the `else` 
part used in stage2 (and as you said, is loaded many times).  Splitting this 
into two files won't make much difference in that regard.

> For bootstrap builds where you want the stage1 to run on your build host, you 
> should be able to set `BOOTSTRAP_CMAKE_TOOLCHAIN_FILE` in the first stage 
> build, to signal to the first stage build that the second stage will be 
> cross-compiled, and we can customize the multi-stage dependencies correctly 
> based on that. That avoids the need for the `ADDITIONAL_CLANG_BOOTSTRAP_DEPS` 
> variable, which feels a bit hacky to me.

Unless there's another way to do it, It's not hacky.  I believe the term is 
`escape hatch`.

While I'm happy to use `BOOTSTRAP_CMAKE_TOOLCHAIN_FILE` instead of passing 
`-DCMAKE_TOOLCHAIN_FILE=${CMAKE_CURRENT_LIST_FILE}`, I do not see how it helps 
with this problem.  When running `ninja stage2`, I need to insure that the 
dependancies where built.  `BOOTSTRAP_LLVM_ENABLE_LLD` can be used to add `lld` 
to the dependency list, but since I'm setting `CLANG_DEFAULT_LINKER=llb`, I 
don't want clang adding `-fuse-ld`.

If I run this on Linux for both stages, it doesn't matter, because 
clang/CMakeLists.txt add llvm-ar and llvm-ranlib automatically, but since I'm 
on APPLE (see clang/CMakeLists.txt:559), they don't get added.

So, how else would I add them?




Comment at: cmake/caches/linux-toolchain.cmake:2
+# This file is intended to support cross compiling a linux toolchain
+# on any host system, includind Darwin.
+#

smeenai wrote:
> Cross-compilation terminology is kinda weird, and traditionally, the "host" 
> is actually the system the built binaries will be run on (Linux in this 
> case), whereas the build machine is the "build" (but of course that word is 
> super ambiguous). I think LLVM generally sticks to that terminology though, 
> e.g. `LLVM_HOST_TRIPLE`.
I'll work on cleaning up this comment, but the idea is that we cross compile on 
any host system, e.g., Linux, Darwin, Windows, etc., and target Linux.



Comment at: cmake/caches/linux-toolchain.cmake:84
+  else()
+set(BOOTSTRAP_STAGE2_PROJECTS "clang;libcxx;libcxxabi;libunwind" CACHE 
STRING "" FORCE)
+  endif()

smeenai wrote:
> Nit: write this out as a list instead of a string with semicolons? (I know 
> they're equivalent, but the list reads nicer IMO.)
Perhaps, but this is the style used throughout the clang+llvm cmake files.



Comment at: cmake/caches/linux-toolchain.cmake:88
+  # Required on non-elf hosts.
+  set(ADDITIONAL_CLANG_BOOTSTRAP_DEPS "lld;llvm-ar;llvm-ranlib" CACHE STRING 
"")
+

smeenai wrote:
> Not exactly related, but I wonder why the LLVM build needs ranlib (rather 
> than just invoking ar appropriately).
Darwin version of ranlib doesn't like elf binaries, so we need the one we build 
in stage1.



Comment at: cmake/caches/linux-toolchain.cmake:102
+else()
+  set(CMAKE_SYSTEM_NAME Linux CACHE STRING "" FORCE)
+

smeenai wrote:
> The CMake documentation for CMAKE_SYSTEM_NAME says CMAKE_SYSTEM_VERSION 
> should also be set when cross-compiling (though I haven't seen any ill 
> effects from not doing so). Setting CMAKE_SYSTEM_PROCESSOR probably doesn't 
> hurt either.
These can be passed to stage1 as BOOTSTRAP_CMAKE_SYSTEM_VERSION, etc., allowing 
the user full control.  I'll add a note to the comments up top.


Repository:
  rC Clang

https://reviews.llvm.org/D41660



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

[PATCH] D41660: [cmake] Add new linux toolchain file

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In https://reviews.llvm.org/D41660#965935, @hintonda wrote:

> In https://reviews.llvm.org/D41660#965877, @beanz wrote:
>
> > You should split the CMake cache file you created into two files, (1) a 
> > CMake Cache to manage the build configuration and (2) a tool chain file for 
> > targeting Linux. As @semeenai pointed out we absolutly want the behavior of 
> > the toolchain file being loaded multiple times. That is the correct way 
> > this build should work.
>
>
> I really like keeping this in a single file, but will break it up if 
> necessary.
>
> The `if` part of the if/else is used in stage1 as a cache file, and the 
> `else` part used in stage2 (and as you said, is loaded many times).  
> Splitting this into two files won't make much difference in that regard.
>
> > For bootstrap builds where you want the stage1 to run on your build host, 
> > you should be able to set `BOOTSTRAP_CMAKE_TOOLCHAIN_FILE` in the first 
> > stage build, to signal to the first stage build that the second stage will 
> > be cross-compiled, and we can customize the multi-stage dependencies 
> > correctly based on that. That avoids the need for the 
> > `ADDITIONAL_CLANG_BOOTSTRAP_DEPS` variable, which feels a bit hacky to me.
>
> Unless there's another way to do it, It's not hacky.  I believe the term is 
> `escape hatch`.
>
> While I'm happy to use `BOOTSTRAP_CMAKE_TOOLCHAIN_FILE` instead of passing 
> `-DCMAKE_TOOLCHAIN_FILE=${CMAKE_CURRENT_LIST_FILE}`, I do not see how it 
> helps with this problem.  When running `ninja stage2`, I need to insure that 
> the dependancies where built.  `BOOTSTRAP_LLVM_ENABLE_LLD` can be used to add 
> `lld` to the dependency list, but since I'm setting 
> `CLANG_DEFAULT_LINKER=llb`, I don't want clang adding `-fuse-ld`.


Did a quick test and setting BOOTSTRAP_CMAKE_TOOLCHAIN_FILE does not work in 
this case.

> If I run this on Linux for both stages, it doesn't matter, because 
> clang/CMakeLists.txt add llvm-ar and llvm-ranlib automatically, but since I'm 
> on APPLE (see clang/CMakeLists.txt:559), they don't get added.
> 
> So, how else would I add them?




Repository:
  rC Clang

https://reviews.llvm.org/D41660



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

[PATCH] D41660: [cmake] Add new linux toolchain file

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In https://reviews.llvm.org/D41660#966023, @phosek wrote:

> @hintonda I think this should be a platform file in 
> https://github.com/llvm-mirror/llvm/tree/master/cmake/platforms rather than 
> Clang cache file. Platform files are concerned with the host platform 
> (including cross-compilation), while cache files are  related to the 
> distribution setup. What you're trying to do is the former rather than the 
> latter. Some of the aspects of your setup like the bootstrap tool setup is 
> already handled by the 2-stage build so you should use that rather than 
> reimplementing your own solution.


Thanks for the pointer.  I'll rework the patch along the lines you suggest.


Repository:
  rC Clang

https://reviews.llvm.org/D41660



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

[PATCH] D41660: [cmake] Add new linux toolchain file

[Don Hinton via Phabricator via cfe-commits]

hintonda abandoned this revision.
hintonda added a comment.

Thanks for all your suggestions.

Planning to rework and move the toolchain specific part to llvm/cmake/platform 
and abandon the cache part altogether.


Repository:
  rC Clang

https://reviews.llvm.org/D41660



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

[PATCH] D41807: [cmake] Delete redundant install command for clang-refactor.

[Don Hinton via Phabricator via cfe-commits]

hintonda created this revision.
hintonda added reviewers: arphaman, beanz.
Herald added a subscriber: mgorny.

Delete redundant install command for clang-refactor.  Install
targets for clang tools are controled by CLANG_BUILD_TOOLS, and when
OFF, cmake issues the following warning:

WARNING: Target "clang-refactor" has EXCLUDE_FROM_ALL set and will not
be built by default but an install rule has been provided for it.
CMake does not define behavior for this case.


Repository:
  rC Clang

https://reviews.llvm.org/D41807

Files:
  tools/clang-refactor/CMakeLists.txt


Index: tools/clang-refactor/CMakeLists.txt
===
--- tools/clang-refactor/CMakeLists.txt
+++ tools/clang-refactor/CMakeLists.txt
@@ -20,5 +20,3 @@
   clangToolingCore
   clangToolingRefactor
   )
-
-install(TARGETS clang-refactor RUNTIME DESTINATION bin)


Index: tools/clang-refactor/CMakeLists.txt
===
--- tools/clang-refactor/CMakeLists.txt
+++ tools/clang-refactor/CMakeLists.txt
@@ -20,5 +20,3 @@
   clangToolingCore
   clangToolingRefactor
   )
-
-install(TARGETS clang-refactor RUNTIME DESTINATION bin)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41623: [cmake] [libcxxabi] Fix path problems when cross compiling.

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

ping...


Repository:
  rCXXA libc++abi

https://reviews.llvm.org/D41623



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

[PATCH] D41622: [cmake] [libcxx] Fix path and flag problems when cross compiling.

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

ping...


Repository:
  rCXX libc++

https://reviews.llvm.org/D41622



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

[PATCH] D41621: [cmake] [libunwind] Fix path problems when cross compiling.

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

ping...


https://reviews.llvm.org/D41621



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

[PATCH] D41829: [cmake] Add cache file to bootstrap 2-stage linux cross compile on Darwin.

[Don Hinton via Phabricator via cfe-commits]

hintonda created this revision.
hintonda added reviewers: beanz, compnerd, phosek, smeenai.
Herald added a subscriber: mgorny.

Add cache file to bootstrap 2-stage linux cross compile on
Darwin.


Repository:
  rC Clang

https://reviews.llvm.org/D41829

Files:
  cmake/caches/Linux.cmake


Index: cmake/caches/Linux.cmake
===
--- /dev/null
+++ cmake/caches/Linux.cmake
@@ -0,0 +1,59 @@
+# This file is primarily for cross compiling clang+llvm, et al, for
+# Linux on Darwin, and can be invoked like this:
+#
+#  cmake -GNinja -DBOOTSTRAP_CMAKE_SYSROOT= [OPTIONS] \
+#-DBOOTSTRAP_CMAKE_TOOLCHAIN_FILE=/cmake/platforms/Linux.cmake \
+#-C /cmake/caches/Linux.cmake ../llvm
+
+if(NOT DEFINED BOOTSTRAP_CMAKE_SYSROOT)
+  message(SEND_ERROR "Missing required argument 
-DBOOTSTRAP_CMAKE_SYSROOT=.")
+endif()
+if(NOT DEFINED BOOTSTRAP_CMAKE_TOOLCHAIN_FILE)
+  message(SEND_ERROR "Missing required argument 
-DBOOTSTRAP_CMAKE_TOOLCHAIN_FILE=.")
+endif()
+
+set(LLVM_TARGETS_TO_BUILD Native CACHE STRING "")
+set(CMAKE_BUILD_TYPE RELEASE CACHE STRING "")
+set(LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
+
+set(CLANG_ENABLE_BOOTSTRAP ON CACHE BOOL "")
+
+if(NOT DEFINED BOOTSTRAP_LLVM_DEFAULT_TARGET_TRIPLE)
+  set(BOOTSTRAP_LLVM_DEFAULT_TARGET_TRIPLE "x86_64-unknown-linux-gnu")
+endif()
+
+# Since LLVM_ENABLE_PROJECTS gets passed automatically to the next
+# stage, use another variable to pass the desired projects to stage2.
+if(DEFINED BOOTSTRAP_LLVM_ENABLE_PROJECTS)
+  set(BOOTSTRAP_LLVM_ENABLE_PROJECTS_OVERRIDE 
${BOOTSTRAP_LLVM_ENABLE_PROJECTS} CACHE STRING "" FORCE)
+  unset(BOOTSTRAP_LLVM_ENABLE_PROJECTS CACHE)
+else()
+  set(BOOTSTRAP_LLVM_ENABLE_PROJECTS_OVERRIDE 
"clang;libcxx;libcxxabi;libunwind" CACHE STRING "" FORCE)
+endif()
+
+if(CMAKE_HOST_APPLE)
+  # Make sure at least clang and lld are included.
+  list(APPEND LLVM_ENABLE_PROJECTS clang lld)
+  set(LLVM_ENABLE_PROJECTS ${LLVM_ENABLE_PROJECTS} CACHE STRING "")
+
+  # Passing -fuse-ld=lld is hard for cmake to handle correctly, so
+  # make lld the default linker.
+  set(CLANG_DEFAULT_LINKER lld CACHE STRING "" FORCE)
+  set(BOOTSTRAP_LLVM_ENABLE_LLD ON CACHE BOOL "")
+
+  set(BOOTSTRAP_CMAKE_AR ${CMAKE_BINARY_DIR}/bin/llvm-ar CACHE STRING "")
+  set(BOOTSTRAP_CMAKE_RANLIB ${CMAKE_BINARY_DIR}/bin/llvm-ranlib CACHE STRING 
"")
+  set(BOOTSTRAP_CLANG_TABLEGEN ${CMAKE_BINARY_DIR}/bin/clang-tblgen CACHE 
STRING "")
+  set(BOOTSTRAP_LLVM_TABLEGEN ${CMAKE_BINARY_DIR}/bin/llvm-tblgen CACHE STRING 
"")
+endif()
+
+# Since we just want to build a bootstrap compiler, turn off as much
+# as possible.
+set(LLVM_INCLUDE_DOCS OFF CACHE BOOL "")
+set(LLVM_INCLUDE_EXAMPLES OFF CACHE BOOL "")
+set(LLVM_INCLUDE_RUNTIMES OFF CACHE BOOL "")
+set(LLVM_INCLUDE_TESTS OFF CACHE BOOL "")
+set(LLVM_INCLUDE_UTILS OFF CACHE BOOL "")
+set(CLANG_BUILD_TOOLS OFF CACHE BOOL "")
+set(CLANG_ENABLE_ARCMT OFF CACHE BOOL "")
+set(CLANG_ENABLE_STATIC_ANALYZER OFF CACHE BOOL "")


Index: cmake/caches/Linux.cmake
===
--- /dev/null
+++ cmake/caches/Linux.cmake
@@ -0,0 +1,59 @@
+# This file is primarily for cross compiling clang+llvm, et al, for
+# Linux on Darwin, and can be invoked like this:
+#
+#  cmake -GNinja -DBOOTSTRAP_CMAKE_SYSROOT= [OPTIONS] \
+#-DBOOTSTRAP_CMAKE_TOOLCHAIN_FILE=/cmake/platforms/Linux.cmake \
+#-C /cmake/caches/Linux.cmake ../llvm
+
+if(NOT DEFINED BOOTSTRAP_CMAKE_SYSROOT)
+  message(SEND_ERROR "Missing required argument -DBOOTSTRAP_CMAKE_SYSROOT=.")
+endif()
+if(NOT DEFINED BOOTSTRAP_CMAKE_TOOLCHAIN_FILE)
+  message(SEND_ERROR "Missing required argument -DBOOTSTRAP_CMAKE_TOOLCHAIN_FILE=.")
+endif()
+
+set(LLVM_TARGETS_TO_BUILD Native CACHE STRING "")
+set(CMAKE_BUILD_TYPE RELEASE CACHE STRING "")
+set(LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
+
+set(CLANG_ENABLE_BOOTSTRAP ON CACHE BOOL "")
+
+if(NOT DEFINED BOOTSTRAP_LLVM_DEFAULT_TARGET_TRIPLE)
+  set(BOOTSTRAP_LLVM_DEFAULT_TARGET_TRIPLE "x86_64-unknown-linux-gnu")
+endif()
+
+# Since LLVM_ENABLE_PROJECTS gets passed automatically to the next
+# stage, use another variable to pass the desired projects to stage2.
+if(DEFINED BOOTSTRAP_LLVM_ENABLE_PROJECTS)
+  set(BOOTSTRAP_LLVM_ENABLE_PROJECTS_OVERRIDE ${BOOTSTRAP_LLVM_ENABLE_PROJECTS} CACHE STRING "" FORCE)
+  unset(BOOTSTRAP_LLVM_ENABLE_PROJECTS CACHE)
+else()
+  set(BOOTSTRAP_LLVM_ENABLE_PROJECTS_OVERRIDE "clang;libcxx;libcxxabi;libunwind" CACHE STRING "" FORCE)
+endif()
+
+if(CMAKE_HOST_APPLE)
+  # Make sure at least clang and lld are included.
+  list(APPEND LLVM_ENABLE_PROJECTS clang lld)
+  set(LLVM_ENABLE_PROJECTS ${LLVM_ENABLE_PROJECTS} CACHE STRING "")
+
+  # Passing -fuse-ld=lld is hard for cmake to handle correctly, so
+  # make lld the default linker.
+  set(CLANG_DEFAULT_LINKER lld CACHE STRING "" FORCE)
+  set(BOOTSTRAP_LLVM_ENABLE_LLD ON CACHE BOOL "")
+
+  set(BOOTSTRAP_CMAKE_AR ${CMAKE_BINARY_DIR}/bin/llvm-ar CACHE STRING "")
+  s

[PATCH] D41829: [cmake] Add cache file to bootstrap 2-stage linux cross compile on Darwin.

[Don Hinton via Phabricator via cfe-commits]

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

- Also pass LLVM_CONFIG_EXE.


Repository:
  rC Clang

https://reviews.llvm.org/D41829

Files:
  cmake/caches/Linux.cmake


Index: cmake/caches/Linux.cmake
===
--- /dev/null
+++ cmake/caches/Linux.cmake
@@ -0,0 +1,60 @@
+# This file is primarily for cross compiling clang+llvm, et al, for
+# Linux on Darwin, and can be invoked like this:
+#
+#  cmake -GNinja -DBOOTSTRAP_CMAKE_SYSROOT= [OPTIONS] \
+#-DBOOTSTRAP_CMAKE_TOOLCHAIN_FILE=/cmake/platforms/Linux.cmake \
+#-C /cmake/caches/Linux.cmake ../llvm
+
+if(NOT DEFINED BOOTSTRAP_CMAKE_SYSROOT)
+  message(SEND_ERROR "Missing required argument 
-DBOOTSTRAP_CMAKE_SYSROOT=.")
+endif()
+if(NOT DEFINED BOOTSTRAP_CMAKE_TOOLCHAIN_FILE)
+  message(SEND_ERROR "Missing required argument 
-DBOOTSTRAP_CMAKE_TOOLCHAIN_FILE=.")
+endif()
+
+set(LLVM_TARGETS_TO_BUILD Native CACHE STRING "")
+set(CMAKE_BUILD_TYPE RELEASE CACHE STRING "")
+set(LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
+
+set(CLANG_ENABLE_BOOTSTRAP ON CACHE BOOL "")
+
+if(NOT DEFINED BOOTSTRAP_LLVM_DEFAULT_TARGET_TRIPLE)
+  set(BOOTSTRAP_LLVM_DEFAULT_TARGET_TRIPLE "x86_64-unknown-linux-gnu")
+endif()
+
+# Since LLVM_ENABLE_PROJECTS gets passed automatically to the next
+# stage, use another variable to pass the desired projects to stage2.
+if(DEFINED BOOTSTRAP_LLVM_ENABLE_PROJECTS)
+  set(BOOTSTRAP_LLVM_ENABLE_PROJECTS_OVERRIDE 
${BOOTSTRAP_LLVM_ENABLE_PROJECTS} CACHE STRING "" FORCE)
+  unset(BOOTSTRAP_LLVM_ENABLE_PROJECTS CACHE)
+else()
+  set(BOOTSTRAP_LLVM_ENABLE_PROJECTS_OVERRIDE 
"clang;libcxx;libcxxabi;libunwind" CACHE STRING "" FORCE)
+endif()
+
+if(CMAKE_HOST_APPLE)
+  # Make sure at least clang and lld are included.
+  list(APPEND LLVM_ENABLE_PROJECTS clang lld)
+  set(LLVM_ENABLE_PROJECTS ${LLVM_ENABLE_PROJECTS} CACHE STRING "")
+
+  # Passing -fuse-ld=lld is hard for cmake to handle correctly, so
+  # make lld the default linker.
+  set(CLANG_DEFAULT_LINKER lld CACHE STRING "" FORCE)
+  set(BOOTSTRAP_LLVM_ENABLE_LLD ON CACHE BOOL "")
+
+  set(BOOTSTRAP_CMAKE_AR ${CMAKE_BINARY_DIR}/bin/llvm-ar CACHE STRING "")
+  set(BOOTSTRAP_CMAKE_RANLIB ${CMAKE_BINARY_DIR}/bin/llvm-ranlib CACHE STRING 
"")
+  set(BOOTSTRAP_CLANG_TABLEGEN ${CMAKE_BINARY_DIR}/bin/clang-tblgen CACHE 
STRING "")
+  set(BOOTSTRAP_LLVM_TABLEGEN ${CMAKE_BINARY_DIR}/bin/llvm-tblgen CACHE STRING 
"")
+  set(BOOTSTRAP_LLVM_CONFIG_EXE ${CMAKE_BINARY_DIR}/bin/llvm-config CACHE 
STRING "")
+endif()
+
+# Since we just want to build a bootstrap compiler, turn off as much
+# as possible.
+set(LLVM_INCLUDE_DOCS OFF CACHE BOOL "")
+set(LLVM_INCLUDE_EXAMPLES OFF CACHE BOOL "")
+set(LLVM_INCLUDE_RUNTIMES OFF CACHE BOOL "")
+set(LLVM_INCLUDE_TESTS OFF CACHE BOOL "")
+set(LLVM_INCLUDE_UTILS OFF CACHE BOOL "")
+set(CLANG_BUILD_TOOLS OFF CACHE BOOL "")
+set(CLANG_ENABLE_ARCMT OFF CACHE BOOL "")
+set(CLANG_ENABLE_STATIC_ANALYZER OFF CACHE BOOL "")


Index: cmake/caches/Linux.cmake
===
--- /dev/null
+++ cmake/caches/Linux.cmake
@@ -0,0 +1,60 @@
+# This file is primarily for cross compiling clang+llvm, et al, for
+# Linux on Darwin, and can be invoked like this:
+#
+#  cmake -GNinja -DBOOTSTRAP_CMAKE_SYSROOT= [OPTIONS] \
+#-DBOOTSTRAP_CMAKE_TOOLCHAIN_FILE=/cmake/platforms/Linux.cmake \
+#-C /cmake/caches/Linux.cmake ../llvm
+
+if(NOT DEFINED BOOTSTRAP_CMAKE_SYSROOT)
+  message(SEND_ERROR "Missing required argument -DBOOTSTRAP_CMAKE_SYSROOT=.")
+endif()
+if(NOT DEFINED BOOTSTRAP_CMAKE_TOOLCHAIN_FILE)
+  message(SEND_ERROR "Missing required argument -DBOOTSTRAP_CMAKE_TOOLCHAIN_FILE=.")
+endif()
+
+set(LLVM_TARGETS_TO_BUILD Native CACHE STRING "")
+set(CMAKE_BUILD_TYPE RELEASE CACHE STRING "")
+set(LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
+
+set(CLANG_ENABLE_BOOTSTRAP ON CACHE BOOL "")
+
+if(NOT DEFINED BOOTSTRAP_LLVM_DEFAULT_TARGET_TRIPLE)
+  set(BOOTSTRAP_LLVM_DEFAULT_TARGET_TRIPLE "x86_64-unknown-linux-gnu")
+endif()
+
+# Since LLVM_ENABLE_PROJECTS gets passed automatically to the next
+# stage, use another variable to pass the desired projects to stage2.
+if(DEFINED BOOTSTRAP_LLVM_ENABLE_PROJECTS)
+  set(BOOTSTRAP_LLVM_ENABLE_PROJECTS_OVERRIDE ${BOOTSTRAP_LLVM_ENABLE_PROJECTS} CACHE STRING "" FORCE)
+  unset(BOOTSTRAP_LLVM_ENABLE_PROJECTS CACHE)
+else()
+  set(BOOTSTRAP_LLVM_ENABLE_PROJECTS_OVERRIDE "clang;libcxx;libcxxabi;libunwind" CACHE STRING "" FORCE)
+endif()
+
+if(CMAKE_HOST_APPLE)
+  # Make sure at least clang and lld are included.
+  list(APPEND LLVM_ENABLE_PROJECTS clang lld)
+  set(LLVM_ENABLE_PROJECTS ${LLVM_ENABLE_PROJECTS} CACHE STRING "")
+
+  # Passing -fuse-ld=lld is hard for cmake to handle correctly, so
+  # make lld the default linker.
+  set(CLANG_DEFAULT_LINKER lld CACHE STRING "" FORCE)
+  set(BOOTSTRAP_LLVM_ENABLE_LLD ON CACHE BOOL "")
+
+  set(BOOTSTRAP_CMAKE_AR ${CMAKE_BINARY_DIR}/bin/llvm-ar CACHE STRING "")
+  set(BO

[PATCH] D59529: Refactor cast<>'s in if conditionals, which can only assert on failure.

[Don Hinton via Phabricator via cfe-commits]

hintonda created this revision.
hintonda added a reviewer: rjmccall.
Herald added a reviewer: martong.
Herald added a reviewer: shafik.
Herald added a project: clang.

This patch refactors several instances of cast<> used in if
conditionals.  Since cast<> asserts on failure, the else branch can
never be taken.

In some cases, the fix is to replace cast<> with dyn_cast<>.  While
others required the removal of the conditional and some minor
refactoring.

A discussion can be seen here: 
http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20190318/265044.html


Repository:
  rC Clang

https://reviews.llvm.org/D59529

Files:
  lib/AST/ASTImporter.cpp
  lib/AST/DeclBase.cpp
  lib/CodeGen/CGExprConstant.cpp
  lib/CodeGen/MicrosoftCXXABI.cpp
  lib/Sema/SemaOpenMP.cpp

Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -10901,7 +10901,7 @@
 for (NamedDecl *D : ULE->decls()) {
   if (D == PrevD)
 Lookups.push_back(UnresolvedSet<8>());
-  else if (auto *DRD = cast(D))
+  else if (auto *DRD = dyn_cast(D))
 Lookups.back().addDecl(DRD);
   PrevD = D;
 }
Index: lib/CodeGen/MicrosoftCXXABI.cpp
===
--- lib/CodeGen/MicrosoftCXXABI.cpp
+++ lib/CodeGen/MicrosoftCXXABI.cpp
@@ -735,7 +735,7 @@
 CGM.CreateRuntimeFunction(FTy, "_CxxThrowException");
 // _CxxThrowException is stdcall on 32-bit x86 platforms.
 if (CGM.getTarget().getTriple().getArch() == llvm::Triple::x86) {
-  if (auto *Fn = cast(Throw.getCallee()))
+  if (auto *Fn = dyn_cast(Throw.getCallee()))
 Fn->setCallingConv(llvm::CallingConv::X86_StdCall);
 }
 return Throw;
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -1698,31 +1698,20 @@
   auto offset = getOffset();
 
   // If we're producing a pointer, this is easy.
-  if (auto destPtrTy = cast(destTy)) {
-if (Value.isNullPointer()) {
-  // FIXME: integer offsets from non-zero null pointers.
-  return CGM.getNullPointer(destPtrTy, DestType);
-}
-
-// Convert the integer to a pointer-sized integer before converting it
-// to a pointer.
-// FIXME: signedness depends on the original integer type.
-auto intptrTy = CGM.getDataLayout().getIntPtrType(destPtrTy);
-llvm::Constant *C = offset;
-C = llvm::ConstantExpr::getIntegerCast(getOffset(), intptrTy,
-   /*isSigned*/ false);
-C = llvm::ConstantExpr::getIntToPtr(C, destPtrTy);
-return C;
-  }
-
-  // Otherwise, we're basically returning an integer constant.
-
-  // FIXME: this does the wrong thing with ptrtoint of a null pointer,
-  // but since we don't know the original pointer type, there's not much
-  // we can do about it.
-
-  auto C = getOffset();
-  C = llvm::ConstantExpr::getIntegerCast(C, destTy, /*isSigned*/ false);
+  auto destPtrTy = cast(destTy);
+  if (Value.isNullPointer()) {
+// FIXME: integer offsets from non-zero null pointers.
+return CGM.getNullPointer(destPtrTy, DestType);
+  }
+
+  // Convert the integer to a pointer-sized integer before converting it
+  // to a pointer.
+  // FIXME: signedness depends on the original integer type.
+  auto intptrTy = CGM.getDataLayout().getIntPtrType(destPtrTy);
+  llvm::Constant *C = offset;
+  C = llvm::ConstantExpr::getIntegerCast(getOffset(), intptrTy,
+ /*isSigned*/ false);
+  C = llvm::ConstantExpr::getIntToPtr(C, destPtrTy);
   return C;
 }
 
Index: lib/AST/DeclBase.cpp
===
--- lib/AST/DeclBase.cpp
+++ lib/AST/DeclBase.cpp
@@ -1179,13 +1179,15 @@
 return this;
 
   case Decl::ObjCInterface:
-if (auto *Def = cast(this)->getDefinition())
-  return Def;
+if (auto *OID = dyn_cast(this))
+  if (auto *Def = OID->getDefinition())
+return Def;
 return this;
 
   case Decl::ObjCProtocol:
-if (auto *Def = cast(this)->getDefinition())
-  return Def;
+if (auto *OPD = dyn_cast(this))
+  if (auto *Def = OPD->getDefinition())
+return Def;
 return this;
 
   case Decl::ObjCCategory:
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -8460,44 +8460,41 @@
   if (!To)
 return llvm::make_error();
 
-  if (auto *FromDC = cast(From)) {
-ASTNodeImporter Importer(*this);
+  auto *FromDC = cast(From);
+  ASTNodeImporter Importer(*this);
 
-if (auto *ToRecord = dyn_cast(To)) {
-  if (!ToRecord->getDefinition()) {
-return Importer.ImportDefinition(
-cast(FromDC), ToRecord,
-ASTNodeImporter::IDK_Everything);
-  }
+  if (auto *ToRecord = dyn_cast(To

[PATCH] D17407: [Sema] PR25755 Fix crash when initializing out-of-order struct references

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 191828.
hintonda added a comment.
Herald added a subscriber: jdoerfert.
Herald added a project: clang.

- Initial checkin from original D17407 .
- Refactored code and simplify tests based on review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D17407

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/test/SemaCXX/cxx0x-initializer-constructor.cpp


Index: clang/test/SemaCXX/cxx0x-initializer-constructor.cpp
===
--- clang/test/SemaCXX/cxx0x-initializer-constructor.cpp
+++ clang/test/SemaCXX/cxx0x-initializer-constructor.cpp
@@ -409,3 +409,21 @@
 [0] = 1, [2] = 3
   }; // expected-error {{ambiguous}} expected-note {{in implicit 
initialization of array element 1}}
 }
+
+namespace PR23514 {
+struct Q {  // expected-note-re 4{{candidate constructor (the implicit 
{{(copy|move)}} constructor) not viable: requires 1 argument, but 0 were 
provided}}
+  Q(int) {} // expected-note 2{{candidate constructor not viable: requires 1 
argument, but 0 were provided}}
+};
+struct A {
+  Q x, y; // expected-note 2{{in implicit initialization of field 'x' with 
omitted initializer}}
+};
+struct B {
+  int i;
+  A a;
+  int k;
+};
+B w = {.a.y = 0, 0, .a.x = 10};
+B x = {.a.y = 0, 0}; // expected-error {{no matching constructor for 
initialization of 'PR23514::Q'}}
+B y = {.a = {.y = 0, .x = 0}, 0};
+B z = {.a = {.y = 0}, 0}; // expected-error {{no matching constructor for 
initialization of 'PR23514::Q'}}
+} // namespace PR23514
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -2000,6 +2000,9 @@
 !IList->isIdiomaticZeroInitializer(SemaRef.getLangOpts());
   bool HasDesignatedInit = false;
 
+  unsigned FieldSize = std::distance(RD->field_begin(), RD->field_end());
+  llvm::BitVector SeenFields(FieldSize);
+
   while (Index < IList->getNumInits()) {
 Expr *Init = IList->getInit(Index);
 SourceLocation InitLoc = Init->getBeginLoc();
@@ -2032,6 +2035,10 @@
 }
   }
 
+  if (!hadError)
+SeenFields.set(Field == FieldEnd ? FieldSize - 1
+ : Field->getFieldIndex() - 1);
+
   InitializedSomething = true;
 
   // Disable check for missing fields when designators are used.
@@ -2045,6 +2052,8 @@
   break;
 }
 
+SeenFields.set(Field->getFieldIndex());
+
 // We've already initialized a member of a union. We're done.
 if (InitializedSomething && DeclType->isUnionType())
   break;
@@ -2111,12 +2120,15 @@
 }
   }
 
-  // Check that any remaining fields can be value-initialized.
-  if (VerifyOnly && Field != FieldEnd && !DeclType->isUnionType() &&
-  !Field->getType()->isIncompleteArrayType()) {
-// FIXME: Should check for holes left by designated initializers too.
-for (; Field != FieldEnd && !hadError; ++Field) {
-  if (!Field->isUnnamedBitfield() && !Field->hasInClassInitializer())
+  // Check that any unseen fields can be value-initialized.
+  if (VerifyOnly && TopLevelObject && !DeclType->isUnionType() &&
+  (Field != FieldEnd || HasDesignatedInit)) {
+Field = RD->field_begin();
+for (unsigned i = 0; i < FieldSize && !hadError; ++i, ++Field) {
+  if (SeenFields[i])
+continue;
+  if(!Field->isUnnamedBitfield() && !Field->hasInClassInitializer() &&
+  !Field->getType()->isIncompleteArrayType())
 CheckEmptyInitializable(
 InitializedEntity::InitializeMember(*Field, &Entity),
 IList->getEndLoc());


Index: clang/test/SemaCXX/cxx0x-initializer-constructor.cpp
===
--- clang/test/SemaCXX/cxx0x-initializer-constructor.cpp
+++ clang/test/SemaCXX/cxx0x-initializer-constructor.cpp
@@ -409,3 +409,21 @@
 [0] = 1, [2] = 3
   }; // expected-error {{ambiguous}} expected-note {{in implicit initialization of array element 1}}
 }
+
+namespace PR23514 {
+struct Q {  // expected-note-re 4{{candidate constructor (the implicit {{(copy|move)}} constructor) not viable: requires 1 argument, but 0 were provided}}
+  Q(int) {} // expected-note 2{{candidate constructor not viable: requires 1 argument, but 0 were provided}}
+};
+struct A {
+  Q x, y; // expected-note 2{{in implicit initialization of field 'x' with omitted initializer}}
+};
+struct B {
+  int i;
+  A a;
+  int k;
+};
+B w = {.a.y = 0, 0, .a.x = 10};
+B x = {.a.y = 0, 0}; // expected-error {{no matching constructor for initialization of 'PR23514::Q'}}
+B y = {.a = {.y = 0, .x = 0}, 0};
+B z = {.a = {.y = 0}, 0}; // expected-error {{no matching constructor for initialization of 'PR23514::Q'}}
+} // namespace PR23514
Index: clang/lib/Sema/SemaInit.cpp
=

[PATCH] D17407: [Sema] PR25755 Handle out of order designated initializers

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D17407#1439029 , @xbolva00 wrote:

> Please check also https://bugs.llvm.org/show_bug.cgi?id=40030


Thanks for pointing that out.  I was mainly handling asserts, but will add the 
restrictions as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D17407



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

[PATCH] D17407: [Sema] PR25755 Handle out of order designated initializers

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

I took a look at C++ designator restrictions shown here aggregate 
initialization 
, and 
believe it to be orthogonal to this change.  Thus, I'd prefer to put that in a 
separate patch.

However, based on this, I also believe I should move the tests into 
test/Sema/designated-initializers.c.

Please let me know if that sounds like the right thing to do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D17407



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

[PATCH] D17407: [Sema] PR25755 Handle out of order designated initializers

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D17407#1439029 , @xbolva00 wrote:

> Please check also https://bugs.llvm.org/show_bug.cgi?id=40030


If you compile with -pedantic, you'll get the following warning:  `warning: 
designated initializers are a C99 feature [-Wc99-extensions]`

This warning currently applies for all versions of C++, but should probably not 
warn for cases supported in c++20.  However, that's a bigger fix, since it 
would need to discriminate between what c++20 does and does not support, e.g., 
nested and out of order designators are not included in c++20.

@rsmith This is ready for review when you get a chance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D17407



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

[PATCH] D59746: [LibTooling] Fix '-h' option

[Don Hinton via Phabricator via cfe-commits]

hintonda created this revision.
hintonda added a reviewer: alexfh.
Herald added a project: clang.

LibTooling's '-h' option was intended to be as an alias for
'-help', but since the '-help' option is declared as a function scoped
static, it isn't visible, leaving it to individual clients to handle
'-h' -- which none seem to do.

This fix scans argv and expands '-h' to '-help' so that llvm's
CommandLineParser can handle it appropriately.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59746

Files:
  clang/lib/Tooling/CommonOptionsParser.cpp


Index: clang/lib/Tooling/CommonOptionsParser.cpp
===
--- clang/lib/Tooling/CommonOptionsParser.cpp
+++ clang/lib/Tooling/CommonOptionsParser.cpp
@@ -114,8 +114,24 @@
   if (!ErrorMessage.empty())
 ErrorMessage.append("\n");
   llvm::raw_string_ostream OS(ErrorMessage);
+
+  // Expand "-h" to "-help" so it's static handler can be used.
+  SmallVector NewArgv;
+  // Append options from command line.
+  bool SawDoubleDash = false;
+  for (int I = 0; I < argc; ++I) {
+StringRef Arg(argv[I]);
+if (Arg == "--")
+  SawDoubleDash = true;
+if(!SawDoubleDash && Arg == "-h")
+  NewArgv.push_back("-help");
+else
+  NewArgv.push_back(argv[I]);
+  }
+  int NewArgc = static_cast(NewArgv.size());
+
   // Stop initializing if command-line option parsing failed.
-  if (!cl::ParseCommandLineOptions(argc, argv, Overview, &OS)) {
+  if (!cl::ParseCommandLineOptions(NewArgc, &NewArgv[0], Overview, &OS)) {
 OS.flush();
 return llvm::make_error("[CommonOptionsParser]: " +
ErrorMessage,


Index: clang/lib/Tooling/CommonOptionsParser.cpp
===
--- clang/lib/Tooling/CommonOptionsParser.cpp
+++ clang/lib/Tooling/CommonOptionsParser.cpp
@@ -114,8 +114,24 @@
   if (!ErrorMessage.empty())
 ErrorMessage.append("\n");
   llvm::raw_string_ostream OS(ErrorMessage);
+
+  // Expand "-h" to "-help" so it's static handler can be used.
+  SmallVector NewArgv;
+  // Append options from command line.
+  bool SawDoubleDash = false;
+  for (int I = 0; I < argc; ++I) {
+StringRef Arg(argv[I]);
+if (Arg == "--")
+  SawDoubleDash = true;
+if(!SawDoubleDash && Arg == "-h")
+  NewArgv.push_back("-help");
+else
+  NewArgv.push_back(argv[I]);
+  }
+  int NewArgc = static_cast(NewArgv.size());
+
   // Stop initializing if command-line option parsing failed.
-  if (!cl::ParseCommandLineOptions(argc, argv, Overview, &OS)) {
+  if (!cl::ParseCommandLineOptions(NewArgc, &NewArgv[0], Overview, &OS)) {
 OS.flush();
 return llvm::make_error("[CommonOptionsParser]: " +
ErrorMessage,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59746: [LibTooling] Fix '-h' option

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

A better alternative would have been to add a cl::aliasopt for '-h' in llvm's 
CommandLineParser when '-help' was first added.  However, that's no longer 
possible since some llvm based tools already use '-h' for other purposes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



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

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

[Don Hinton via Phabricator via cfe-commits]

hintonda created this revision.
hintonda added reviewers: rsmith, xbolva00.
Herald added a project: clang.

Basic designator initializers are allowed in c++2a, so only
issue warnings for the extended ones that aren't allowed:

  - out of order
- nested
- arrays
- mixed


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59754

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp

Index: clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
===
--- clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
+++ clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++2a %s -verify
+// RUN: %clang_cc1 -std=c++2a %s -verify -pedantic
 
 namespace class_with_ctor {
   struct A { // expected-note 6{{candidate}}
@@ -21,3 +21,20 @@
   C c1 = {{}, {}}; // ok, call default ctor twice
   C c2 = {{1, 2}, {3, 4}}; // expected-error 2{{no matching constructor}}
 }
+
+namespace designator {
+struct A { int x, y; };
+struct B { A a; };
+
+// out of order designators
+A a1 = {.y = 1, .x = 2}; // expected-warning {{designated initializers are a C99 feature}}
+
+// array designator
+int arr[3] = {[1] = 5}; // expected-warning {{designated initializers are a C99 feature}}
+
+// nested designator
+B b = {.a.x = 0}; // expected-warning {{designated initializers are a C99 feature}}
+
+// mixed designator and non-designator
+A a2 = {.x = 1, 2}; // expected-warning {{designated initializers are a C99 feature}}
+}
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -1999,6 +1999,7 @@
   bool CheckForMissingFields =
 !IList->isIdiomaticZeroInitializer(SemaRef.getLangOpts());
   bool HasDesignatedInit = false;
+  bool HasNonDesginatedInit = false;
 
   while (Index < IList->getNumInits()) {
 Expr *Init = IList->getInit(Index);
@@ -2013,6 +2014,10 @@
 
   HasDesignatedInit = true;
 
+  auto LastIdx = Field != FieldEnd
+ ? Field->getFieldIndex()
+ : std::distance(RD->field_begin(), RD->field_end());
+
   // Handle this designated initializer. Field will be updated to
   // the next field that we'll be initializing.
   if (CheckDesignatedInitializer(Entity, IList, DIE, 0,
@@ -2032,6 +2037,15 @@
 }
   }
 
+  // Warn on out of order and mixed designators in C++20.
+  auto NextIdx = Field != FieldEnd
+ ? Field->getFieldIndex()
+ : std::distance(RD->field_begin(), RD->field_end());
+  if (VerifyOnly && (LastIdx >= NextIdx || HasNonDesginatedInit) &&
+  SemaRef.getLangOpts().CPlusPlus2a)
+SemaRef.Diag(Init->getBeginLoc(), diag::ext_designated_init)
+<< Init->getSourceRange();
+
   InitializedSomething = true;
 
   // Disable check for missing fields when designators are used.
@@ -2045,6 +2059,11 @@
   break;
 }
 
+// Warn on mixed designators in C++20
+if (VerifyOnly && HasDesignatedInit && SemaRef.getLangOpts().CPlusPlus2a)
+  SemaRef.Diag(Init->getBeginLoc(), diag::ext_designated_init)
+  << Init->getSourceRange();
+
 // We've already initialized a member of a union. We're done.
 if (InitializedSomething && DeclType->isUnionType())
   break;
@@ -2976,6 +2995,7 @@
   bool Invalid = false;
   SmallVector Designators;
   SmallVector InitExpressions;
+  bool HaveArrayDesignator = false;
 
   // Build designators and check array designator expressions.
   for (unsigned Idx = 0; Idx < Desig.getNumDesignators(); ++Idx) {
@@ -2999,6 +3019,7 @@
 D.getRBracketLoc()));
 InitExpressions.push_back(Index);
   }
+  HaveArrayDesignator = true;
   break;
 }
 
@@ -3042,6 +3063,7 @@
   InitExpressions.push_back(EndIndex);
 }
   }
+  HaveArrayDesignator = true;
   break;
 }
 }
@@ -3060,8 +3082,11 @@
  Init.getAs());
 
   if (!getLangOpts().C99)
-Diag(DIE->getBeginLoc(), diag::ext_designated_init)
-<< DIE->getSourceRange();
+// Warn on nested and array designators in C++20
+if (!getLangOpts().CPlusPlus2a || Desig.getNumDesignators() > 1 ||
+HaveArrayDesignator)
+  Diag(DIE->getBeginLoc(), diag::ext_designated_init)
+  << DIE->getSourceRange();
 
   return DIE;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Don Hinton via Phabricator via cfe-commits]

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

- Fix variable typo/spellings, and add missing flag assignment and additional 
test to catch it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59754

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp

Index: clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
===
--- clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
+++ clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++2a %s -verify
+// RUN: %clang_cc1 -std=c++2a %s -verify -pedantic
 
 namespace class_with_ctor {
   struct A { // expected-note 6{{candidate}}
@@ -21,3 +21,21 @@
   C c1 = {{}, {}}; // ok, call default ctor twice
   C c2 = {{1, 2}, {3, 4}}; // expected-error 2{{no matching constructor}}
 }
+
+namespace designator {
+struct A { int x, y; };
+struct B { A a; };
+
+// out of order designators
+A a1 = {.y = 1, .x = 2}; // expected-warning {{designated initializers are a C99 feature}}
+
+// array designator
+int arr[3] = {[1] = 5}; // expected-warning {{designated initializers are a C99 feature}}
+
+// nested designator
+B b = {.a.x = 0}; // expected-warning {{designated initializers are a C99 feature}}
+
+// mixed designator and non-designator
+A a2 = {.x = 1, 2}; // expected-warning {{designated initializers are a C99 feature}}
+A a3 = {1, .y = 2}; // expected-warning {{designated initializers are a C99 feature}}
+}
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -1999,6 +1999,7 @@
   bool CheckForMissingFields =
 !IList->isIdiomaticZeroInitializer(SemaRef.getLangOpts());
   bool HasDesignatedInit = false;
+  bool HasNonDesignatedInit = false;
 
   while (Index < IList->getNumInits()) {
 Expr *Init = IList->getInit(Index);
@@ -2013,6 +2014,10 @@
 
   HasDesignatedInit = true;
 
+  auto LastIdx = Field != FieldEnd
+ ? Field->getFieldIndex()
+ : std::distance(RD->field_begin(), RD->field_end());
+
   // Handle this designated initializer. Field will be updated to
   // the next field that we'll be initializing.
   if (CheckDesignatedInitializer(Entity, IList, DIE, 0,
@@ -2032,6 +2037,15 @@
 }
   }
 
+  // Warn on out of order and mixed designators in C++20.
+  auto NextIdx = Field != FieldEnd
+ ? Field->getFieldIndex()
+ : std::distance(RD->field_begin(), RD->field_end());
+  if (VerifyOnly && (LastIdx >= NextIdx || HasNonDesignatedInit) &&
+  SemaRef.getLangOpts().CPlusPlus2a)
+SemaRef.Diag(Init->getBeginLoc(), diag::ext_designated_init)
+<< Init->getSourceRange();
+
   InitializedSomething = true;
 
   // Disable check for missing fields when designators are used.
@@ -2045,6 +2059,13 @@
   break;
 }
 
+HasNonDesignatedInit = true;
+
+// Warn on mixed designators in C++20
+if (VerifyOnly && HasDesignatedInit && SemaRef.getLangOpts().CPlusPlus2a)
+  SemaRef.Diag(Init->getBeginLoc(), diag::ext_designated_init)
+  << Init->getSourceRange();
+
 // We've already initialized a member of a union. We're done.
 if (InitializedSomething && DeclType->isUnionType())
   break;
@@ -2976,6 +2997,7 @@
   bool Invalid = false;
   SmallVector Designators;
   SmallVector InitExpressions;
+  bool HasArrayDesignator = false;
 
   // Build designators and check array designator expressions.
   for (unsigned Idx = 0; Idx < Desig.getNumDesignators(); ++Idx) {
@@ -2999,6 +3021,7 @@
 D.getRBracketLoc()));
 InitExpressions.push_back(Index);
   }
+  HasArrayDesignator = true;
   break;
 }
 
@@ -3042,6 +3065,7 @@
   InitExpressions.push_back(EndIndex);
 }
   }
+  HasArrayDesignator = true;
   break;
 }
 }
@@ -3060,8 +3084,11 @@
  Init.getAs());
 
   if (!getLangOpts().C99)
-Diag(DIE->getBeginLoc(), diag::ext_designated_init)
-<< DIE->getSourceRange();
+// Warn on nested and array designators in C++20
+if (!getLangOpts().CPlusPlus2a || Desig.getNumDesignators() > 1 ||
+HasArrayDesignator)
+  Diag(DIE->getBeginLoc(), diag::ext_designated_init)
+  << DIE->getSourceRange();
 
   return DIE;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Don Hinton via Phabricator via cfe-commits]

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

Thanks for the review.




Comment at: clang/lib/Sema/SemaInit.cpp:2017
 
+  auto LastIdx = Field != FieldEnd
+ ? Field->getFieldIndex()

Rakete wrote:
> Can `Field` ever be `FieldEnd` at this point?
Yes.  If the last trip through the loop was a designated initializer, and it 
happened to be the last field in the RecordDecl, Field will be equal FieldEnd.  
At this point, the loop will exit below unless there's another designated 
initializer that's out of order.



Comment at: clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp:30
+// out of order designators
+A a1 = {.y = 1, .x = 2}; // expected-warning {{designated initializers are a 
C99 feature}}
+

Rakete wrote:
> Those warnings are misleading, since C++20 does have designated initializers; 
> they just don't support some stuff that C99 does. It would be better  IMO if 
> you could separate them. As in, the above should give you: `out-of-order 
> designated initializers are a C99 feature` or something like that.
I think that would be a good idea as well, but wanted to get advise first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59754



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

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

[Don Hinton via Phabricator via cfe-commits]

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



Comment at: clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp:30
+// out of order designators
+A a1 = {.y = 1, .x = 2}; // expected-warning {{designated initializers are a 
C99 feature}}
+

lebedev.ri wrote:
> hintonda wrote:
> > Rakete wrote:
> > > Those warnings are misleading, since C++20 does have designated 
> > > initializers; they just don't support some stuff that C99 does. It would 
> > > be better  IMO if you could separate them. As in, the above should give 
> > > you: `out-of-order designated initializers are a C99 feature` or 
> > > something like that.
> > I think that would be a good idea as well, but wanted to get advise first.
> > As in, the above should give you: out-of-order designated initializers are 
> > a C99 feature or something like that.
> 
> I suppose also the question is, whether to error-out, or support them as an 
> extension?
> 
Although most of them seem fine, the nested ones can be problematic.  Please 
see https://reviews.llvm.org/D17407 for a proposal on how to fix them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59754



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

hintonda created this revision.
hintonda added reviewers: alexfh, rjmccall.
Herald added subscribers: xazax.hun, mgorny.
Herald added a project: clang.

Finds cases of cast<> used as the condition of a conditional
statements which will assert on failure in Debug builds.  The use of
cast<> implies that the operation cannot fail, and should not be used
as the condition of a conditional statement..

.. code-block:: c++

  // Finds cases like these:
  if (auto x = cast(y)) <...>
  if (cast(y)) <...>
  
  // But not cases like these:
  if (auto f = cast(y)->foo()) <...>
  if (cast(y)->foo()) <...>


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59802

Files:
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.h
  clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
  clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp

Index: clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
@@ -0,0 +1,25 @@
+// RUN: %check_clang_tidy %s llvm-avoid-cast-in-conditional %t
+
+struct X;
+struct Y;
+struct Z {
+  int foo();
+};
+
+template 
+X* cast(Y*);
+
+bool foo(Y* y) {
+  if (auto x = cast(y))
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: {{.*llvm-avoid-cast-in-conditional}}
+return true;
+  if (cast(y))
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning:  {{.*llvm-avoid-cast-in-conditional}}
+return true;
+
+  // Does not trigger warning.
+  if (cast(y)->foo())
+return true;
+  return false;
+}
+
Index: clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
@@ -0,0 +1,19 @@
+.. title:: clang-tidy - llvm-avoid-cast-in-conditional
+
+llvm-avoid-cast-in-conditional
+==
+
+Finds cases of cast<> used as the condition of a conditional
+statements which will assert on failure in Debug builds.  The use of
+cast<> implies that the operation cannot fail, and should not be used
+as the condition of a conditional statement..
+
+.. code-block:: c++
+
+  // Finds cases like these:
+  if (auto x = cast(y)) <...>
+  if (cast(y)) <...>
+
+  // But not cases like these:
+  if (auto f = cast(y)->foo()) <...>
+  if (cast(y)->foo()) <...>
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -175,6 +175,7 @@
hicpp-use-nullptr (redirects to modernize-use-nullptr) 
hicpp-use-override (redirects to modernize-use-override) 
hicpp-vararg (redirects to cppcoreguidelines-pro-type-vararg) 
+   llvm-avoid-cast-in-conditional
llvm-header-guard
llvm-include-order
llvm-namespace-comment
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -130,6 +130,12 @@
   ` now supports `OverrideSpelling`
   and `FinalSpelling` options.
 
+- New :doc:`llvm-avoid-cast-in-conditional
+  ` check.
+
+  Finds cases of cast<> used as condition in conditional statements which will
+  assert on failure in Debug builds.
+
 - New :doc:`openmp-exception-escape
   ` check.
 
Index: clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
===
--- clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
+++ clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
@@ -14,11 +14,13 @@
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringRef.h"
 
+using llvm::SmallSet;
+
 namespace clang {
 namespace tidy {
 namespace utils {
 
-typedef llvm::SmallSet HeaderFileExtensionsSet;
+typedef SmallSet HeaderFileExtensionsSet;
 
 /// \brief Checks whether expansion location of \p Loc is in header file.
 bool isExpansionLocInHeaderFile(
Index: clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
+++ clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "../readability/NamespaceCommentCheck.h"
+#include "AvoidCastInConditionalCheck.h"
 #include "HeaderG

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h:23
 
-typedef llvm::SmallSet HeaderFileExtensionsSet;
+typedef SmallSet HeaderFileExtensionsSet;
 

Eugene.Zelenko wrote:
> Please use using. See modernize-use-using.
Normally I'd agree, but since the llvm checkers live in the llvm namespace, 
including any llvm checker prior to this header will prevent clang from finding 
`llvm::SmallSet`.  As you can see, the other llvm classes don't need the 
`llvm::` prefix, but that's because `clang/include/clang/Basic/LLVM.h` already 
has a bunch of `using` statements.

So, this code won't compile without this change, or the addition of `using 
llvm::SmallSet;` to `clang/include/clang/Basic/LLVM.h`.  Should I make that 
change first?  Then just remove the `llvm::` prefix?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h:23
 
-typedef llvm::SmallSet HeaderFileExtensionsSet;
+typedef SmallSet HeaderFileExtensionsSet;
 

hintonda wrote:
> Eugene.Zelenko wrote:
> > Please use using. See modernize-use-using.
> Normally I'd agree, but since the llvm checkers live in the llvm namespace, 
> including any llvm checker prior to this header will prevent clang from 
> finding `llvm::SmallSet`.  As you can see, the other llvm classes don't need 
> the `llvm::` prefix, but that's because `clang/include/clang/Basic/LLVM.h` 
> already has a bunch of `using` statements.
> 
> So, this code won't compile without this change, or the addition of `using 
> llvm::SmallSet;` to `clang/include/clang/Basic/LLVM.h`.  Should I make that 
> change first?  Then just remove the `llvm::` prefix?
Oh, sorry, didn't read closely enough.  I can get rid of the typedef.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

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

[Don Hinton via Phabricator via cfe-commits]

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



Comment at: clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp:30
+// out of order designators
+A a1 = {.y = 1, .x = 2}; // expected-warning {{designated initializers are a 
C99 feature}}
+

Rakete wrote:
> hintonda wrote:
> > lebedev.ri wrote:
> > > hintonda wrote:
> > > > Rakete wrote:
> > > > > Those warnings are misleading, since C++20 does have designated 
> > > > > initializers; they just don't support some stuff that C99 does. It 
> > > > > would be better  IMO if you could separate them. As in, the above 
> > > > > should give you: `out-of-order designated initializers are a C99 
> > > > > feature` or something like that.
> > > > I think that would be a good idea as well, but wanted to get advise 
> > > > first.
> > > > As in, the above should give you: out-of-order designated initializers 
> > > > are a C99 feature or something like that.
> > > 
> > > I suppose also the question is, whether to error-out, or support them as 
> > > an extension?
> > > 
> > Although most of them seem fine, the nested ones can be problematic.  
> > Please see https://reviews.llvm.org/D17407 for a proposal on how to fix 
> > them.
> > I suppose also the question is, whether to error-out, or support them as an 
> > extension?
> 
> Yes that's true. gcc doesn't support them at all in C++, and it seems like we 
> accept it as well, but only for C classes (constructors make clang crash).
> 
> But making it an error now breaks backwards compatibility. So I think the 
> best solution is to accept it for now, as an extension.
Btw, several cpp tests use nested designated initializers.  Here's a quick list 
I got by temporarily making it an error:

  CodeGenCXX/mangle-exprs.cpp
  SemaCXX/cxx2a-initializer-aggregates.cpp
  SemaCXX/decltype.cpp
  SemaTemplate/instantiate-init.cpp

I believe there are a several out of order ones as well, but those might have 
been unintentional.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59754



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

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

[Don Hinton via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaInit.cpp:2044
+ : std::distance(RD->field_begin(), RD->field_end());
+  if (VerifyOnly && (LastIdx >= NextIdx || HasNonDesignatedInit) &&
+  SemaRef.getLangOpts().CPlusPlus2a)

Rakete wrote:
> `!VerifyOnly` and braces please.
Will commit this change as soon as check-clang finishes, but not sure I grok 
the VerityOnly/!VerifyOnly criteria.  Could you help me out? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59754



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

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

[Don Hinton via Phabricator via cfe-commits]

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

- Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59754

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp

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

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

[Don Hinton via Phabricator via cfe-commits]

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

- Missed one set of braces.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59754

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp

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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

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

- Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802

Files:
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.h
  clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
  clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp

Index: clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
@@ -0,0 +1,25 @@
+// RUN: %check_clang_tidy %s llvm-avoid-cast-in-conditional %t
+
+struct X;
+struct Y;
+struct Z {
+  int foo();
+};
+
+template 
+X* cast(Y*);
+
+bool foo(Y* y) {
+  if (auto x = cast(y))
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: {{.*llvm-avoid-cast-in-conditional}}
+return true;
+  if (cast(y))
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning:  {{.*llvm-avoid-cast-in-conditional}}
+return true;
+
+  // Does not trigger warning.
+  if (cast(y)->foo())
+return true;
+  return false;
+}
+
Index: clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
@@ -0,0 +1,19 @@
+.. title:: clang-tidy - llvm-avoid-cast-in-conditional
+
+llvm-avoid-cast-in-conditional
+==
+
+Finds cases of `cast<>` used as the condition of a conditional
+statements which will assert on failure in Debug builds.  The use of
+cast<> implies that the operation cannot fail, and should not be used
+as the condition of a conditional statement.
+
+.. code-block:: c++
+
+  // Finds cases like these:
+  if (auto x = cast(y)) <...>
+  if (cast(y)) <...>
+
+  // But not cases like these:
+  if (auto f = cast(y)->foo()) <...>
+  if (cast(y)->foo()) <...>
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -175,6 +175,7 @@
hicpp-use-nullptr (redirects to modernize-use-nullptr) 
hicpp-use-override (redirects to modernize-use-override) 
hicpp-vararg (redirects to cppcoreguidelines-pro-type-vararg) 
+   llvm-avoid-cast-in-conditional
llvm-header-guard
llvm-include-order
llvm-namespace-comment
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -130,6 +130,12 @@
   ` now supports `OverrideSpelling`
   and `FinalSpelling` options.
 
+- New :doc:`llvm-avoid-cast-in-conditional
+  ` check.
+
+  Finds cases of `cast<>` used as condition in conditional statements
+  which will assert on failure in Debug builds.
+
 - New :doc:`openmp-exception-escape
   ` check.
 
Index: clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
===
--- clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
+++ clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
@@ -14,11 +14,13 @@
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringRef.h"
 
+using llvm::SmallSet;
+
 namespace clang {
 namespace tidy {
 namespace utils {
 
-typedef llvm::SmallSet HeaderFileExtensionsSet;
+using HeaderFileExtensionsSet = SmallSet;
 
 /// \brief Checks whether expansion location of \p Loc is in header file.
 bool isExpansionLocInHeaderFile(
Index: clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
+++ clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "../readability/NamespaceCommentCheck.h"
+#include "AvoidCastInConditionalCheck.h"
 #include "HeaderGuardCheck.h"
 #include "IncludeOrderCheck.h"
 #include "TwineLocalCheck.h"
@@ -21,6 +22,8 @@
 class LLVMModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+CheckFactories.registerCheck(
+"llvm-avoid-cast-in-conditional");
 CheckFactories.registerCheck("llvm-header-guard");
 CheckFactories.registerCheck("llvm-include-order");
 CheckFactories.

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h:17
 
+using llvm::SmallSet;
+

Eugene.Zelenko wrote:
> It'll be easier to use llvm::SmallSet in using statement.
Unfortunately, that won't compile.

The reason is that the llvm checkers live in `namespace 
clang::clang-tidy::llvm`, so if you include an llvm checker header, which opens 
the `clang::clang-tidy::llvm` namespace, clang doesn't know whether to look in 
`llvm` or `clang::clang-tidy::llvm` since you are in `clang::clang-tidy`.

The error does suggest using `::llvm::SmallSet`, but that seems sorta klugey.

Since this looks ugly, how about I add `using llvm::SmallSet;` to ` 
clang/include/clang/LLVM.h`?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h:17
 
+using llvm::SmallSet;
+

hintonda wrote:
> Eugene.Zelenko wrote:
> > It'll be easier to use llvm::SmallSet in using statement.
> Unfortunately, that won't compile.
> 
> The reason is that the llvm checkers live in `namespace 
> clang::clang-tidy::llvm`, so if you include an llvm checker header, which 
> opens the `clang::clang-tidy::llvm` namespace, clang doesn't know whether to 
> look in `llvm` or `clang::clang-tidy::llvm` since you are in 
> `clang::clang-tidy`.
> 
> The error does suggest using `::llvm::SmallSet`, but that seems sorta klugey.
> 
> Since this looks ugly, how about I add `using llvm::SmallSet;` to ` 
> clang/include/clang/LLVM.h`?
> 
Btw, the reason this never caused a problem before was that the first llvm 
checker header included in 
`clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp` was 
`HeaderGuardCheck.h`, which only include `HeaderGuardCheck.h` before opening 
the new `clang::clang-tidy::llvm` namespace.  Once I added this checker, which 
was included before `HeaderGuardCheck.h`, it made the new `llvm` namespace was 
ambiguous within `clang-tidy`, hence the error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

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

- Address additional comments, and move  to LLVM.h.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802

Files:
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.h
  clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
  clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
  clang/include/clang/Basic/LLVM.h

Index: clang/include/clang/Basic/LLVM.h
===
--- clang/include/clang/Basic/LLVM.h
+++ clang/include/clang/Basic/LLVM.h
@@ -31,6 +31,7 @@
   template class ArrayRef;
   template class MutableArrayRef;
   template class OwningArrayRef;
+  template  class SmallSet;
   template class SmallString;
   template class SmallVector;
   template class SmallVectorImpl;
@@ -66,6 +67,7 @@
   using llvm::Optional;
   using llvm::OwningArrayRef;
   using llvm::SaveAndRestore;
+  using llvm::SmallSet;
   using llvm::SmallString;
   using llvm::SmallVector;
   using llvm::SmallVectorImpl;
Index: clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
@@ -0,0 +1,25 @@
+// RUN: %check_clang_tidy %s llvm-avoid-cast-in-conditional %t
+
+struct X;
+struct Y;
+struct Z {
+  int foo();
+};
+
+template 
+X* cast(Y*);
+
+bool foo(Y* y) {
+  if (auto x = cast(y))
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: {{.*llvm-avoid-cast-in-conditional}}
+return true;
+  if (cast(y))
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning:  {{.*llvm-avoid-cast-in-conditional}}
+return true;
+
+  // Does not trigger warning.
+  if (cast(y)->foo())
+return true;
+  return false;
+}
+
Index: clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
@@ -0,0 +1,19 @@
+.. title:: clang-tidy - llvm-avoid-cast-in-conditional
+
+llvm-avoid-cast-in-conditional
+==
+
+Finds cases of ``cast<>`` used as the condition of a conditional
+statements which will assert on failure in Debug builds. The use of
+cast<> implies that the operation cannot fail, and should not be used
+as the condition of a conditional statement.
+
+.. code-block:: c++
+
+  // Finds cases like these:
+  if (auto x = cast(y)) <...>
+  if (cast(y)) <...>
+
+  // But not cases like these:
+  if (auto f = cast(y)->foo()) <...>
+  if (cast(y)->foo()) <...>
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -175,6 +175,7 @@
hicpp-use-nullptr (redirects to modernize-use-nullptr) 
hicpp-use-override (redirects to modernize-use-override) 
hicpp-vararg (redirects to cppcoreguidelines-pro-type-vararg) 
+   llvm-avoid-cast-in-conditional
llvm-header-guard
llvm-include-order
llvm-namespace-comment
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -130,6 +130,12 @@
   ` now supports `OverrideSpelling`
   and `FinalSpelling` options.
 
+- New :doc:`llvm-avoid-cast-in-conditional
+  ` check.
+
+  Finds cases of ``cast<>`` used as condition in conditional statements
+  which will assert on failure in Debug builds.
+
 - New :doc:`openmp-exception-escape
   ` check.
 
Index: clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
===
--- clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
+++ clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
@@ -18,7 +18,7 @@
 namespace tidy {
 namespace utils {
 
-typedef llvm::SmallSet HeaderFileExtensionsSet;
+using HeaderFileExtensionsSet = SmallSet;
 
 /// \brief Checks whether expansion location of \p Loc is in header file.
 bool isExpansionLocInHeaderFile(
Index: clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
+++ clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidyM

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D59802#1443340 , @aaron.ballman 
wrote:

> Should this check also try to find this pattern:
>
>   if (dyn_cast(Bobble))
> ...
>
>
> in order to recommend the proper replacement:
>
>   if (isa(Bobble))
> ...
>
>
> I ask because the name `llvm-avoid-cast-in-conditional` sounds like it would 
> also cover this situation and I run into it during code reviews with some 
> frequency (more often than I run into `cast<>` being used in a conditional).


Yes, I can add that, and provide a fix-it too.  Thanks...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D59802#1443451 , @hintonda wrote:

> In D59802#1443340 , @aaron.ballman 
> wrote:
>
> > Should this check also try to find this pattern:
> >
> >   if (dyn_cast(Bobble))
> > ...
> >
> >
> > in order to recommend the proper replacement:
> >
> >   if (isa(Bobble))
> > ...
> >
> >
> > I ask because the name `llvm-avoid-cast-in-conditional` sounds like it 
> > would also cover this situation and I run into it during code reviews with 
> > some frequency (more often than I run into `cast<>` being used in a 
> > conditional).
>
>
> Yes, I can add that, and provide a fix-it too.  Thanks...


I did a quick grep and found a few of these, but also found the `_or_null<>` 
variety.  Guess they'll need to stay the same since `isa<>` can't handle nulls.

Btw, I also found the same pattern used for `while()`, so I'll add that too.  
Here's a sample of the patterns I'm seeing:

./lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp:213:  
while (dyn_cast(last_stmt)) {
./clang/lib/CodeGen/CodeGenModule.cpp:1390:  if 
(dyn_cast_or_null(D)) . // <--- this one's okay
./clang/lib/CodeGen/CodeGenModule.cpp:3950:if 
(dyn_cast(callSite)) {


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D59802#1443527 , @aaron.ballman 
wrote:

> Would it make sense to transform `if (dyn_cast_or_null(Obj))` into `if 
> (Obj && isa(Obj))`  or are there bad transformations from that?


Sure, that sounds reasonable.  I only see about 12 cases across all repos, so 
it isn't that common.  Whereas the idiom you present is used quite often.

I haven't looked yet, but wouldn't the use of `isa_or_null<>` be more efficient 
in cases like this?

./clang/lib/Sema/AnalysisBasedWarnings.cpp:401:  if (B->getTerminator() 
&& isa(B->getTerminator()))
./clang/lib/AST/Expr.cpp:2734:if (MCE->getMethodDecl() && 
isa(MCE->getMethodDecl()))

Granted, there aren't a lot of these.

> 
> 
>> Btw, I also found the same pattern used for `while()`, so I'll add that too. 
>>  Here's a sample of the patterns I'm seeing:
>> 
>> ./lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp:213:  
>> while (dyn_cast(last_stmt)) {
> 
> Hah, good catch!
> 
>> ./clang/lib/CodeGen/CodeGenModule.cpp:1390:  if 
>> (dyn_cast_or_null(D)) . // <--- this one's okay
> 
> I think this could be expressed as `if (D && isa(D))`, no?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

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

- Address additional comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802

Files:
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.h
  clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
  clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
  clang/include/clang/Basic/LLVM.h

Index: clang/include/clang/Basic/LLVM.h
===
--- clang/include/clang/Basic/LLVM.h
+++ clang/include/clang/Basic/LLVM.h
@@ -31,6 +31,7 @@
   template class ArrayRef;
   template class MutableArrayRef;
   template class OwningArrayRef;
+  template  class SmallSet;
   template class SmallString;
   template class SmallVector;
   template class SmallVectorImpl;
@@ -66,6 +67,7 @@
   using llvm::Optional;
   using llvm::OwningArrayRef;
   using llvm::SaveAndRestore;
+  using llvm::SmallSet;
   using llvm::SmallString;
   using llvm::SmallVector;
   using llvm::SmallVectorImpl;
Index: clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
@@ -0,0 +1,25 @@
+// RUN: %check_clang_tidy %s llvm-avoid-cast-in-conditional %t
+
+struct X;
+struct Y;
+struct Z {
+  int foo();
+};
+
+template 
+X* cast(Y*);
+
+bool foo(Y* y) {
+  if (auto x = cast(y))
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: {{.*llvm-avoid-cast-in-conditional}}
+return true;
+  if (cast(y))
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning:  {{.*llvm-avoid-cast-in-conditional}}
+return true;
+
+  // Does not trigger warning.
+  if (cast(y)->foo())
+return true;
+  return false;
+}
+
Index: clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
@@ -0,0 +1,19 @@
+.. title:: clang-tidy - llvm-avoid-cast-in-conditional
+
+llvm-avoid-cast-in-conditional
+==
+
+Finds cases of ``cast<>`` used as the condition of a conditional
+statements which will assert on failure in Debug builds. The use of
+``cast<>`` implies that the operation cannot fail, and should not be
+used as the condition of a conditional statement.
+
+.. code-block:: c++
+
+  // Finds cases like these:
+  if (auto x = cast(y)) <...>
+  if (cast(y)) <...>
+
+  // But not cases like these:
+  if (auto f = cast(y)->foo()) <...>
+  if (cast(y)->foo()) <...>
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -175,6 +175,7 @@
hicpp-use-nullptr (redirects to modernize-use-nullptr) 
hicpp-use-override (redirects to modernize-use-override) 
hicpp-vararg (redirects to cppcoreguidelines-pro-type-vararg) 
+   llvm-avoid-cast-in-conditional
llvm-header-guard
llvm-include-order
llvm-namespace-comment
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -130,6 +130,12 @@
   ` now supports `OverrideSpelling`
   and `FinalSpelling` options.
 
+- New :doc:`llvm-avoid-cast-in-conditional
+  ` check.
+
+  Finds cases of ``cast<>`` used as condition in conditional statements
+  which will assert on failure in Debug builds.
+
 - New :doc:`openmp-exception-escape
   ` check.
 
Index: clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
===
--- clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
+++ clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
@@ -18,7 +18,7 @@
 namespace tidy {
 namespace utils {
 
-typedef llvm::SmallSet HeaderFileExtensionsSet;
+using HeaderFileExtensionsSet = SmallSet;
 
 /// \brief Checks whether expansion location of \p Loc is in header file.
 bool isExpansionLocInHeaderFile(
Index: clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
+++ clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidyModule.h"
 #includ

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

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

- Added dyn_cast and dyn_cast_or_null, and provided fixit's for all of them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802

Files:
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.h
  clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
  clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
  clang/include/clang/Basic/LLVM.h

Index: clang/include/clang/Basic/LLVM.h
===
--- clang/include/clang/Basic/LLVM.h
+++ clang/include/clang/Basic/LLVM.h
@@ -31,6 +31,7 @@
   template class ArrayRef;
   template class MutableArrayRef;
   template class OwningArrayRef;
+  template  class SmallSet;
   template class SmallString;
   template class SmallVector;
   template class SmallVectorImpl;
@@ -66,6 +67,7 @@
   using llvm::Optional;
   using llvm::OwningArrayRef;
   using llvm::SaveAndRestore;
+  using llvm::SmallSet;
   using llvm::SmallString;
   using llvm::SmallVector;
   using llvm::SmallVectorImpl;
Index: clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
@@ -0,0 +1,102 @@
+// RUN: %check_clang_tidy %s llvm-avoid-cast-in-conditional %t
+
+struct X;
+struct Y;
+struct Z {
+  int foo();
+  X *bar();
+};
+
+template 
+bool isa(Y *);
+template 
+X *cast(Y *);
+template 
+X *dyn_cast(Y *);
+template 
+X *dyn_cast_or_null(Y *);
+
+bool foo(Y *y, Z *LongVarName) {
+  if (auto x = cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (auto x = dyn_cast(y))
+
+  if (cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (isa(y))
+
+  if (dyn_cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (isa(y))
+
+  if (dyn_cast_or_null(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*dyn_cast_or_null<> not used.*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (y && isa(y))
+
+  if (dyn_cast_or_null(LongVarName->bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*dyn_cast_or_null<> not used.*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (LongVarName->bar() && isa(LongVarName->bar()))
+
+  // These don't trigger a warning.
+  if (auto x = cast(y)->foo())
+return true;
+  if (cast(y)->foo())
+return true;
+
+  while (auto x = cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:19: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (auto x = dyn_cast(y))
+
+  while (cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y))
+
+  while (dyn_cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y))
+
+  while (dyn_cast_or_null(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning:  {{.*dyn_cast_or_null<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (y && isa(y))
+
+  // These don't trigger a warning.
+  while (auto x = cast(y)->foo())
+break;
+  while (cast(y)->foo())
+break;
+
+  do {
+break;
+  } while (cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y));
+
+  do {
+break;
+  } while (dyn_cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y));
+
+  do {
+break;
+  } while (dyn_cast_or_null(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{.*dyn_cast_or_null<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (y && isa(y))
+
+  do {
+break;
+  } while (dyn_cast_or_null(LongVarName->bar()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{.*dyn_cast_or_null<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (LongVarName->bar() && isa(LongVarName->bar()))
+
+  return false;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

I looked at the IR generated at `-O2`, and found that while `if (isa(y))` is 
a modest win over `if (dyn_cast(y)`,  `if (dyn_cast_or_null(y))` 
generates exactly the same IR that `if(y && isa(y))` does.  Also, if `y` is 
actually an expression that makes a function call, it's more expensive because 
it will make the call twice.

So I don't seen any reason to replace `dyn_cast_or_null<>` in conditionals.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

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

- Removed the dyn_cast_or_null replacements.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802

Files:
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.h
  clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
  clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
  clang/include/clang/Basic/LLVM.h

Index: clang/include/clang/Basic/LLVM.h
===
--- clang/include/clang/Basic/LLVM.h
+++ clang/include/clang/Basic/LLVM.h
@@ -31,6 +31,7 @@
   template class ArrayRef;
   template class MutableArrayRef;
   template class OwningArrayRef;
+  template  class SmallSet;
   template class SmallString;
   template class SmallVector;
   template class SmallVectorImpl;
@@ -66,6 +67,7 @@
   using llvm::Optional;
   using llvm::OwningArrayRef;
   using llvm::SaveAndRestore;
+  using llvm::SmallSet;
   using llvm::SmallString;
   using llvm::SmallVector;
   using llvm::SmallVectorImpl;
Index: clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
@@ -0,0 +1,74 @@
+// RUN: %check_clang_tidy %s llvm-avoid-cast-in-conditional %t
+
+struct X;
+struct Y;
+struct Z {
+  int foo();
+};
+
+template 
+bool isa(Y *);
+template 
+X *cast(Y *);
+template 
+X *dyn_cast(Y *);
+template 
+X *dyn_cast_or_null(Y *);
+
+bool foo(Y *y) {
+  if (auto x = cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (auto x = dyn_cast(y))
+
+  if (cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (isa(y))
+
+  if (dyn_cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (isa(y))
+
+  // These don't trigger a warning.
+  if (auto x = cast(y)->foo())
+return true;
+  if (cast(y)->foo())
+return true;
+
+  while (auto x = cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:19: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (auto x = dyn_cast(y))
+
+  while (cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y))
+
+  while (dyn_cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y))
+
+  // These don't trigger a warning.
+  while (auto x = cast(y)->foo())
+break;
+  while (cast(y)->foo())
+break;
+
+  do {
+break;
+  } while (cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y));
+
+  do {
+break;
+  } while (dyn_cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y));
+
+  return false;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
@@ -0,0 +1,28 @@
+.. title:: clang-tidy - llvm-avoid-cast-in-conditional
+
+llvm-avoid-cast-in-conditional
+==
+
+  Finds uses of ``cast<>`` in conditionals of ``if``, ``while`` or
+  ``do`` statements, which will assert rather than return a null
+  pointer. It also finds uses of ``dyn_cast<>`` in conditionals where
+  the return value is not captured.
+
+.. code-block:: c++
+
+  // Finds these:
+  if (auto x = cast(y)) {}
+  // is replaced by:
+  if (auto x = dyn_cast(y)) {}
+
+  if (cast(y)) {}
+  // is replaced by:
+  if (isa(y)) {}
+
+  if (dyn_cast(y)) {}
+  // is replaced by:
+  if (isa(y)) {}
+
+  // These are ignored.
+  if (auto f = cast(y)->foo()) {}
+  if (cast(y)->foo()) {}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -175,6 +175,7 @@
hicpp-use-nullptr (re

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

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

- Add replacement of `y && cast(y)` with `dyn_cast_or_null(y)`, which is 
at least as efficient, and can be a big win if `y` is a function call.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802

Files:
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.h
  clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
  clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
  clang/include/clang/Basic/LLVM.h

Index: clang/include/clang/Basic/LLVM.h
===
--- clang/include/clang/Basic/LLVM.h
+++ clang/include/clang/Basic/LLVM.h
@@ -31,6 +31,7 @@
   template class ArrayRef;
   template class MutableArrayRef;
   template class OwningArrayRef;
+  template  class SmallSet;
   template class SmallString;
   template class SmallVector;
   template class SmallVectorImpl;
@@ -66,6 +67,7 @@
   using llvm::Optional;
   using llvm::OwningArrayRef;
   using llvm::SaveAndRestore;
+  using llvm::SmallSet;
   using llvm::SmallString;
   using llvm::SmallVector;
   using llvm::SmallVectorImpl;
Index: clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
@@ -0,0 +1,96 @@
+// RUN: %check_clang_tidy %s llvm-avoid-cast-in-conditional %t
+
+struct X;
+struct Y;
+struct Z {
+  int foo();
+  X *bar();
+};
+
+template 
+bool isa(Y *);
+template 
+X *cast(Y *);
+template 
+X *dyn_cast(Y *);
+template 
+X *dyn_cast_or_null(Y *);
+
+bool foo(Y *y) {
+  if (auto x = cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (auto x = dyn_cast(y))
+
+  if (cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (isa(y))
+
+  if (dyn_cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (isa(y))
+
+  // These don't trigger a warning.
+  if (auto x = cast(y)->foo())
+return true;
+  if (cast(y)->foo())
+return true;
+
+  while (auto x = cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:19: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (auto x = dyn_cast(y))
+
+  while (cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y))
+
+  while (dyn_cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y))
+
+  // These don't trigger a warning.
+  while (auto x = cast(y)->foo())
+break;
+  while (cast(y)->foo())
+break;
+
+  do {
+break;
+  } while (cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y));
+
+  do {
+break;
+  } while (dyn_cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y));
+
+  if (y && cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*use dyn_cast_or_null .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (dyn_cast_or_null(y))
+
+  Z z;
+  if (z.bar() && cast(z.bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*use dyn_cast_or_null .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (dyn_cast_or_null(z.bar()))
+
+  bool b = y && cast(y);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{.*use dyn_cast_or_null .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: bool b = dyn_cast_or_null(y);
+
+  // These don't trigger a warning.
+  if (y && cast(z.bar())) {
+return true;
+  }
+  bool b2 = y && cast(&z);
+
+  return false;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
@@ -0,0 +1,28 @@
+.. title:: clang-tidy - llvm-avoid-cast-in-conditional
+
+llvm-avoid-cast-in-conditional
+==
+
+  Finds uses of ``cast<>`` in

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

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

- Remove commented code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802

Files:
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.h
  clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
  clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
  clang/include/clang/Basic/LLVM.h

Index: clang/include/clang/Basic/LLVM.h
===
--- clang/include/clang/Basic/LLVM.h
+++ clang/include/clang/Basic/LLVM.h
@@ -31,6 +31,7 @@
   template class ArrayRef;
   template class MutableArrayRef;
   template class OwningArrayRef;
+  template  class SmallSet;
   template class SmallString;
   template class SmallVector;
   template class SmallVectorImpl;
@@ -66,6 +67,7 @@
   using llvm::Optional;
   using llvm::OwningArrayRef;
   using llvm::SaveAndRestore;
+  using llvm::SmallSet;
   using llvm::SmallString;
   using llvm::SmallVector;
   using llvm::SmallVectorImpl;
Index: clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
@@ -0,0 +1,96 @@
+// RUN: %check_clang_tidy %s llvm-avoid-cast-in-conditional %t
+
+struct X;
+struct Y;
+struct Z {
+  int foo();
+  X *bar();
+};
+
+template 
+bool isa(Y *);
+template 
+X *cast(Y *);
+template 
+X *dyn_cast(Y *);
+template 
+X *dyn_cast_or_null(Y *);
+
+bool foo(Y *y) {
+  if (auto x = cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (auto x = dyn_cast(y))
+
+  if (cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (isa(y))
+
+  if (dyn_cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (isa(y))
+
+  // These don't trigger a warning.
+  if (auto x = cast(y)->foo())
+return true;
+  if (cast(y)->foo())
+return true;
+
+  while (auto x = cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:19: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (auto x = dyn_cast(y))
+
+  while (cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y))
+
+  while (dyn_cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y))
+
+  // These don't trigger a warning.
+  while (auto x = cast(y)->foo())
+break;
+  while (cast(y)->foo())
+break;
+
+  do {
+break;
+  } while (cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y));
+
+  do {
+break;
+  } while (dyn_cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y));
+
+  if (y && cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*use dyn_cast_or_null .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (dyn_cast_or_null(y))
+
+  Z z;
+  if (z.bar() && cast(z.bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*use dyn_cast_or_null .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (dyn_cast_or_null(z.bar()))
+
+  bool b = y && cast(y);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{.*use dyn_cast_or_null .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: bool b = dyn_cast_or_null(y);
+
+  // These don't trigger a warning.
+  if (y && cast(z.bar())) {
+return true;
+  }
+  bool b2 = y && cast(&z);
+
+  return false;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
@@ -0,0 +1,28 @@
+.. title:: clang-tidy - llvm-avoid-cast-in-conditional
+
+llvm-avoid-cast-in-conditional
+==
+
+  Finds uses of ``cast<>`` in conditionals of ``if``, ``while`` or
+  ``do`` statements, which will assert rather than return a null
+  pointer. It a

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

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






Comment at: clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:73
+  Result.Nodes.getNodeAs("cast-assign")) {
+auto StartLoc = MatchedDecl->getCallee()->getExprLoc();
+auto EndLoc = StartLoc.getLocWithOffset(StringRef("cast").size() - 1);

Eugene.Zelenko wrote:
> Please don't use auto where type is not obvious.
Happy to make the change, but thought the `get.*Loc()` methods were obvious?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D59802#1447127 , @Eugene.Zelenko 
wrote:

> In D59802#1447108 , @hintonda wrote:
>
> >
>
>
> It's good idea to follow modernize-use-auto convention without introducing 
> exceptions.


No worries.  I'm just not sure I fully grok them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

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

- Remove spurious auto's.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802

Files:
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.h
  clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
  clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
  clang/include/clang/Basic/LLVM.h

Index: clang/include/clang/Basic/LLVM.h
===
--- clang/include/clang/Basic/LLVM.h
+++ clang/include/clang/Basic/LLVM.h
@@ -31,6 +31,7 @@
   template class ArrayRef;
   template class MutableArrayRef;
   template class OwningArrayRef;
+  template  class SmallSet;
   template class SmallString;
   template class SmallVector;
   template class SmallVectorImpl;
@@ -66,6 +67,7 @@
   using llvm::Optional;
   using llvm::OwningArrayRef;
   using llvm::SaveAndRestore;
+  using llvm::SmallSet;
   using llvm::SmallString;
   using llvm::SmallVector;
   using llvm::SmallVectorImpl;
Index: clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
@@ -0,0 +1,96 @@
+// RUN: %check_clang_tidy %s llvm-avoid-cast-in-conditional %t
+
+struct X;
+struct Y;
+struct Z {
+  int foo();
+  X *bar();
+};
+
+template 
+bool isa(Y *);
+template 
+X *cast(Y *);
+template 
+X *dyn_cast(Y *);
+template 
+X *dyn_cast_or_null(Y *);
+
+bool foo(Y *y) {
+  if (auto x = cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (auto x = dyn_cast(y))
+
+  if (cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (isa(y))
+
+  if (dyn_cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (isa(y))
+
+  // These don't trigger a warning.
+  if (auto x = cast(y)->foo())
+return true;
+  if (cast(y)->foo())
+return true;
+
+  while (auto x = cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:19: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (auto x = dyn_cast(y))
+
+  while (cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y))
+
+  while (dyn_cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y))
+
+  // These don't trigger a warning.
+  while (auto x = cast(y)->foo())
+break;
+  while (cast(y)->foo())
+break;
+
+  do {
+break;
+  } while (cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y));
+
+  do {
+break;
+  } while (dyn_cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y));
+
+  if (y && cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*use dyn_cast_or_null .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (dyn_cast_or_null(y))
+
+  Z z;
+  if (z.bar() && cast(z.bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*use dyn_cast_or_null .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (dyn_cast_or_null(z.bar()))
+
+  bool b = y && cast(y);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{.*use dyn_cast_or_null .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: bool b = dyn_cast_or_null(y);
+
+  // These don't trigger a warning.
+  if (y && cast(z.bar())) {
+return true;
+  }
+  bool b2 = y && cast(&z);
+
+  return false;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
@@ -0,0 +1,28 @@
+.. title:: clang-tidy - llvm-avoid-cast-in-conditional
+
+llvm-avoid-cast-in-conditional
+==
+
+  Finds uses of ``cast<>`` in conditionals of ``if``, ``while`` or
+  ``do`` statements, which will assert rather than return a null
+  pointer. It 

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

If the `dyn_cast_or_null<>` replacement is acceptable, I'll update the 
documentation and warning message -- suggestions are appreciated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

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

- Add isa and dyn_cast when matching for dyn_cast_or_null replacement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802

Files:
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.h
  clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
  clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
  clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
  clang/include/clang/Basic/LLVM.h

Index: clang/include/clang/Basic/LLVM.h
===
--- clang/include/clang/Basic/LLVM.h
+++ clang/include/clang/Basic/LLVM.h
@@ -31,6 +31,7 @@
   template class ArrayRef;
   template class MutableArrayRef;
   template class OwningArrayRef;
+  template  class SmallSet;
   template class SmallString;
   template class SmallVector;
   template class SmallVectorImpl;
@@ -66,6 +67,7 @@
   using llvm::Optional;
   using llvm::OwningArrayRef;
   using llvm::SaveAndRestore;
+  using llvm::SmallSet;
   using llvm::SmallString;
   using llvm::SmallVector;
   using llvm::SmallVectorImpl;
Index: clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
@@ -0,0 +1,106 @@
+// RUN: %check_clang_tidy %s llvm-avoid-cast-in-conditional %t
+
+struct X;
+struct Y;
+struct Z {
+  int foo();
+  X *bar();
+};
+
+template 
+bool isa(Y *);
+template 
+X *cast(Y *);
+template 
+X *dyn_cast(Y *);
+template 
+X *dyn_cast_or_null(Y *);
+
+bool foo(Y *y) {
+  if (auto x = cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (auto x = dyn_cast(y))
+
+  if (cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (isa(y))
+
+  if (dyn_cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (isa(y))
+
+  // These don't trigger a warning.
+  if (auto x = cast(y)->foo())
+return true;
+  if (cast(y)->foo())
+return true;
+
+  while (auto x = cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:19: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (auto x = dyn_cast(y))
+
+  while (cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y))
+
+  while (dyn_cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y))
+
+  // These don't trigger a warning.
+  while (auto x = cast(y)->foo())
+break;
+  while (cast(y)->foo())
+break;
+
+  do {
+break;
+  } while (cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y));
+
+  do {
+break;
+  } while (dyn_cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y));
+
+  if (y && isa(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*use dyn_cast_or_null .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (dyn_cast_or_null(y))
+
+  if (y && cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*use dyn_cast_or_null .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (dyn_cast_or_null(y))
+
+  if (y && dyn_cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*use dyn_cast_or_null .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (dyn_cast_or_null(y))
+
+  Z z;
+  if (z.bar() && cast(z.bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*use dyn_cast_or_null .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (dyn_cast_or_null(z.bar()))
+
+  bool b = y && cast(y);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{.*use dyn_cast_or_null .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: bool b = dyn_cast_or_null(y);
+
+  // These don't trigger a warning.
+  if (y && cast(z.bar())) {
+return true;
+  }
+  bool b2 = y && cast(&z);
+
+  return false;
+}
Index: clang-tools-extra/docs/clang-tidy/check

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 192844.
hintonda marked 3 inline comments as done.
hintonda added a comment.

- Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802

Files:
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.h
  clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
  clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp

Index: clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
@@ -0,0 +1,118 @@
+// RUN: %check_clang_tidy %s llvm-avoid-cast-in-conditional %t
+
+#define CAST(T, Obj) cast(Obj)
+#define ISA(T, Obj) isa(Obj)
+#define ISA_OR_NULL(T, Obj) Obj && isa(Obj)
+
+struct X;
+struct Y;
+struct Z {
+  int foo();
+  X *bar();
+};
+
+template 
+bool isa(Y *);
+template 
+X *cast(Y *);
+template 
+X *dyn_cast(Y *);
+template 
+X *dyn_cast_or_null(Y *);
+
+bool foo(Y *y, Z *z) {
+  if (auto x = cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (auto x = dyn_cast(y))
+
+  if (cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (isa(y))
+
+  if (dyn_cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (isa(y))
+
+  // These don't trigger a warning.
+  if (auto x = cast(y)->foo())
+return true;
+  if (cast(y)->foo())
+return true;
+
+  while (auto x = cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:19: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (auto x = dyn_cast(y))
+
+  while (cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y))
+
+  while (dyn_cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y))
+
+  // These don't trigger a warning.
+  while (auto x = cast(y)->foo())
+break;
+  while (cast(y)->foo())
+break;
+
+  do {
+break;
+  } while (cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y));
+
+  do {
+break;
+  } while (dyn_cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y));
+
+  if (z->bar() && isa(z->bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*use dyn_cast_or_null .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (dyn_cast_or_null(z->bar()))
+
+  if (z->bar() && cast(z->bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*use dyn_cast_or_null .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (dyn_cast_or_null(z->bar()))
+
+  if (z->bar() && dyn_cast(z->bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*use dyn_cast_or_null .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (dyn_cast_or_null(z->bar()))
+
+  if (z->bar() && dyn_cast_or_null(z->bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*use dyn_cast_or_null .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (dyn_cast_or_null(z->bar()))
+
+  bool b = z->bar() && cast(z->bar());
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{.*use dyn_cast_or_null .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: bool b = dyn_cast_or_null(z->bar());
+
+  // These don't trigger a warning.
+  if (auto x = CAST(X, y))
+return true;
+  if (z->bar() && ISA(Y, z->bar()))
+return true;
+  if (ISA_OR_NULL(Y, z->bar()))
+return true;
+  if (y && isa(y))
+return true;
+  if (y && cast(z->bar()))
+return true;
+  if (z && cast(y)->foo())
+return true;
+  bool b2 = y && cast(z);
+
+  return false;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
@@ -0,0 +1,28 @@
+.. title:: clang-tidy - llvm-avoid-cast-in-conditional
+
+llvm-avoid-cast-in-condition

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D59802#1447560 , @aaron.ballman 
wrote:

> In D59802#1445566 , @hintonda wrote:
>
> > I looked at the IR generated at `-O2`, and found that while `if 
> > (isa(y))` is a modest win over `if (dyn_cast(y)`,  `if 
> > (dyn_cast_or_null(y))` generates exactly the same IR that `if(y && 
> > isa(y))` does.  Also, if `y` is actually an expression that makes a 
> > function call, it's more expensive because it will make the call twice.
> >
> > So I don't seen any reason to replace `dyn_cast_or_null<>` in conditionals.
>
>
> Mostly because I think it's more clear with `isa<>` because that's really 
> what it's checking. However, I could see not doing an automatic transform in 
> the case where the expression argument is something expensive, like a 
> function call. I could also see this as an impetus for adding `isa_or_null<>` 
> as a separate commit.


My last patch only changes `if(y && isa(y))` if `y` is a 
`CXXMemberCallExpr`, so I hope that addresses your concern.




Comment at: 
clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:145
+
+diag(MatchedDecl->getBeginLoc(), "use dyn_cast_or_null")
+<< FixItHint::CreateReplacement(SourceRange(MatchedDecl->getBeginLoc(),

aaron.ballman wrote:
> This diagnostic doesn't tell the user what they've done wrong with the code 
> or why this is a better choice.
Yes, but I'm not yet sure what it should say.  Was sorta hoping for a 
suggestion.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

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

- Improve warning message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802

Files:
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.h
  clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
  clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp

Index: clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
@@ -0,0 +1,118 @@
+// RUN: %check_clang_tidy %s llvm-avoid-cast-in-conditional %t
+
+#define CAST(T, Obj) cast(Obj)
+#define ISA(T, Obj) isa(Obj)
+#define ISA_OR_NULL(T, Obj) Obj &&isa(Obj)
+
+struct X;
+struct Y;
+struct Z {
+  int foo();
+  X *bar();
+};
+
+template 
+bool isa(Y *);
+template 
+X *cast(Y *);
+template 
+X *dyn_cast(Y *);
+template 
+X *dyn_cast_or_null(Y *);
+
+bool foo(Y *y, Z *z) {
+  if (auto x = cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (auto x = dyn_cast(y))
+
+  if (cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (isa(y))
+
+  if (dyn_cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (isa(y))
+
+  // These don't trigger a warning.
+  if (auto x = cast(y)->foo())
+return true;
+  if (cast(y)->foo())
+return true;
+
+  while (auto x = cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:19: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (auto x = dyn_cast(y))
+
+  while (cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y))
+
+  while (dyn_cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y))
+
+  // These don't trigger a warning.
+  while (auto x = cast(y)->foo())
+break;
+  while (cast(y)->foo())
+break;
+
+  do {
+break;
+  } while (cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y));
+
+  do {
+break;
+  } while (dyn_cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y));
+
+  if (z->bar() && isa(z->bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{method .* is called twice .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (dyn_cast_or_null(z->bar()))
+
+  if (z->bar() && cast(z->bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{method .* is called twice .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (dyn_cast_or_null(z->bar()))
+
+  if (z->bar() && dyn_cast(z->bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{method .* is called twice .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (dyn_cast_or_null(z->bar()))
+
+  if (z->bar() && dyn_cast_or_null(z->bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{method .* is called twice .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (dyn_cast_or_null(z->bar()))
+
+  bool b = z->bar() && cast(z->bar());
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{method .* is called twice .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: bool b = dyn_cast_or_null(z->bar());
+
+  // These don't trigger a warning.
+  if (auto x = CAST(X, y))
+return true;
+  if (z->bar() && ISA(Y, z->bar()))
+return true;
+  if (ISA_OR_NULL(Y, z->bar()))
+return true;
+  if (y && isa(y))
+return true;
+  if (y && cast(z->bar()))
+return true;
+  if (z && cast(y)->foo())
+return true;
+  bool b2 = y && cast(z);
+
+  return false;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
@@ -0,0 +1,28 @@
+.. title:: clang-tidy - llvm-avoid-cast-in-conditional
+
+llvm-avoid-cast-in-conditional
+==

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

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

- Added  matcher, and reordered tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802

Files:
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.h
  clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
  clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp

Index: clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
@@ -0,0 +1,118 @@
+// RUN: %check_clang_tidy %s llvm-avoid-cast-in-conditional %t
+
+struct X;
+struct Y;
+struct Z {
+  int foo();
+  X *bar();
+};
+
+template 
+bool isa(Y *);
+template 
+X *cast(Y *);
+template 
+X *dyn_cast(Y *);
+template 
+X *dyn_cast_or_null(Y *);
+
+bool foo(Y *y, Z *z) {
+  if (auto x = cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (auto x = dyn_cast(y))
+
+  while (auto x = cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:19: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (auto x = dyn_cast(y))
+
+  if (cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (isa(y))
+
+  while (cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y))
+
+  do {
+break;
+  } while (cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y));
+
+  if (dyn_cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (isa(y))
+
+  while (dyn_cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y))
+
+  do {
+break;
+  } while (dyn_cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y));
+
+  if (z->bar() && isa(z->bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{method .* is called twice .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (dyn_cast_or_null(z->bar()))
+
+  if (z->bar() && cast(z->bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{method .* is called twice .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (dyn_cast_or_null(z->bar()))
+
+  if (z->bar() && dyn_cast(z->bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{method .* is called twice .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (dyn_cast_or_null(z->bar()))
+
+  if (z->bar() && dyn_cast_or_null(z->bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{method .* is called twice .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (dyn_cast_or_null(z->bar()))
+
+  bool b = z->bar() && cast(z->bar());
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{method .* is called twice .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: bool b = dyn_cast_or_null(z->bar());
+
+  // These don't trigger a warning.
+  if (auto x = cast(y)->foo())
+return true;
+  while (auto x = cast(y)->foo())
+break;
+  if (cast(y)->foo())
+return true;
+  while (cast(y)->foo())
+break;
+  if (y && isa(y))
+return true;
+  if (y && cast(z->bar()))
+return true;
+  if (z && cast(y)->foo())
+return true;
+  bool b2 = y && cast(z);
+
+#define CAST(T, Obj) cast(Obj)
+#define AUTO_VAR_CAST(X, Y, Z) auto X = cast(Z)
+#define ISA(T, Obj) isa(Obj)
+#define ISA_OR_NULL(T, Obj) Obj &&isa(Obj)
+
+  if (auto x = CAST(X, y))
+return true;
+  if (AUTO_VAR_CAST(x, X, z))
+return true;
+  if (z->bar() && ISA(Y, z->bar()))
+return true;
+  if (ISA_OR_NULL(Y, z->bar()))
+return true;
+
+  return false;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
@@ -0,0 +1,28 @@
+.. title:: clang-tidy - llvm-avoid-cast-in-conditional
+
+llvm-avoid-cast-

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D59802#1448574 , @hintonda wrote:

> - Added  matcher, and reordered tests.


Commandline ate my ticks... ;-(

Should be added `isMacroID` matcher.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h:21
 
-typedef llvm::SmallSet HeaderFileExtensionsSet;
+using HeaderFileExtensionsSet = ::llvm::SmallSet;
 

aaron.ballman wrote:
> Can you add back the `::llvm::` qualifier on the `StringRef` type?
I could do that, but do you mean just this case of StringRef, of all four?  And 
what about SourceLocation and SourceManager?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h:21
 
-typedef llvm::SmallSet HeaderFileExtensionsSet;
+using HeaderFileExtensionsSet = ::llvm::SmallSet;
 

aaron.ballman wrote:
> hintonda wrote:
> > aaron.ballman wrote:
> > > Can you add back the `::llvm::` qualifier on the `StringRef` type?
> > I could do that, but do you mean just this case of StringRef, of all four?  
> > And what about SourceLocation and SourceManager?
> If any changes are needed in this file at all, I'd prefer to keep them 
> minimal and self-consistent, so only this instance. However, I wouldn't be 
> opposed to a consistency cleanup in this file as a separate patch with NFC.
Since fixing the bug in this file is orthogonal to this patch, I'll address it 
in it's own patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:125-126
+
+diag(MatchedDecl->getBeginLoc(), "return value from dyn_cast<> not used")
+<< FixItHint::CreateReplacement(SourceRange(StartLoc, EndLoc), "isa");
+  } else if (const auto *MatchedDecl =

aaron.ballman wrote:
> There are zero test cases that seem to trigger this diagnostic text.
Sorry for any confusion, but there are actually 3 tests for this below.  
That'll be made clearer once I address your comments below, and spell out the 
entire diagnostic message on the first instance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:145
+
+diag(MatchedDecl->getBeginLoc(), "use dyn_cast_or_null")
+<< FixItHint::CreateReplacement(SourceRange(MatchedDecl->getBeginLoc(),

aaron.ballman wrote:
> hintonda wrote:
> > aaron.ballman wrote:
> > > This diagnostic doesn't tell the user what they've done wrong with the 
> > > code or why this is a better choice.
> > Yes, but I'm not yet sure what it should say.  Was sorta hoping for a 
> > suggestion.  
> Do you have any evidence that this situation happens in practice? I kind of 
> feel like this entire branch could be eliminated from this patch unless it 
> actually catches problems that happen.
Yes, here are a few from clang/lib -- let me know if you think it's worth it or 
not to keep this:

- DiagnosticName: llvm-avoid-cast-in-conditional
  FileOffset: 305293
  FilePath: 
/Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/Sema/SemaTemplate.cpp
  Message: method 'getAsTemplateDecl' is called twice and could be expensive
  Replacements:
  - FilePath: 
/Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/Sema/SemaTemplate.cpp
Length: 93
Offset: 305293
ReplacementText: 
dyn_cast_or_null(Name.getAsTemplateDecl())
- DiagnosticName: llvm-avoid-cast-in-conditional
  FileOffset: 153442
  FilePath: 
/Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/AST/ASTContext.cpp
  Message: method 'getAsTemplateDecl' is called twice and could be expensive
  Replacements:
  - FilePath: 
/Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/AST/ASTContext.cpp
Length: 92
Offset: 153442
ReplacementText: 
dyn_cast_or_null(Template.getAsTemplateDecl())
- DiagnosticName: llvm-avoid-cast-in-conditional
  FileOffset: 97556
  FilePath: 
/Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/AST/Expr.cpp
  Message: method 'getMethodDecl' is called twice and could be expensive
  Replacements:
  - FilePath: 
/Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/AST/Expr.cpp
Length: 68
Offset: 97556
ReplacementText: dyn_cast_or_null(MCE->getMethodDecl())
- DiagnosticName: llvm-avoid-cast-in-conditional
  FileOffset: 301950
  FilePath: 
/Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/Sema/SemaInit.cpp
  Message: method 'get' is called twice and could be expensive
  Replacements:
  - FilePath: 
/Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/Sema/SemaInit.cpp
Length: 49
Offset: 301950
ReplacementText: dyn_cast_or_null(CurInit.get())
- DiagnosticName: llvm-avoid-cast-in-conditional
  FileOffset: 14335
  FilePath: 
/Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/Sema/AnalysisBasedWarnings.cpp
  Message: method 'operator bool' is called twice and could be expensive
  Replacements:
  - FilePath: 
/Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/Sema/AnalysisBasedWarnings.cpp
Length: 57
Offset: 14335
ReplacementText: dyn_cast_or_null(B->getTerminator())
- DiagnosticName: llvm-avoid-cast-in-conditional
  FileOffset: 15997
  FilePath: 
/Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/Sema/AnalysisBasedWarnings.cpp
  Message: method 'operator bool' is called twice and could be expensive
  Replacements:
  - FilePath: 
/Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/Sema/AnalysisBasedWarnings.cpp
Length: 55
Offset: 15997
ReplacementText: dyn_cast_or_null(B.getTerminator())
- DiagnosticName: llvm-avoid-cast-in-conditional
  FileOffset: 9492
  FilePath: 
/Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
  Message: method 'sexpr' is called twice and could be expensive
  Replacements:
  - FilePath: 
/Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
Length: 39
Offset: 9492
ReplacementText: dyn_cast_or_null(sexpr())
- DiagnosticName: llvm-avoid-cast-in-conditional
  FileOffset: 9572
  FilePath: 
/Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
  Message: method 'sexpr' is called twice and could be expensive
  Replacements:
  - FilePath: 
/Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
Length: 38
Offset: 9572
ReplacementText: dyn_cast_or_null(sexpr())
- DiagnosticName: llvm-avoid-cast-in-conditional
  FileOffset: 9492
  FilePath: 
/Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
  Message: method 'sexpr' is called twice and could be expensive
  Replacements:
  - FilePath: 
/Users/dhinton/projects/llvm_

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

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

- Defer HeaderFileExtionsUtils.h bugfix to separate patch.
- Simplify code per comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802

Files:
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.h
  clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
  clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp

Index: clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
@@ -0,0 +1,118 @@
+// RUN: %check_clang_tidy %s llvm-avoid-cast-in-conditional %t
+
+struct X;
+struct Y;
+struct Z {
+  int foo();
+  X *bar();
+};
+
+template 
+bool isa(Y *);
+template 
+X *cast(Y *);
+template 
+X *dyn_cast(Y *);
+template 
+X *dyn_cast_or_null(Y *);
+
+bool foo(Y *y, Z *z) {
+  if (auto x = cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: cast<> in conditional will assert rather than return a null pointer [llvm-avoid-cast-in-conditional]
+  // CHECK-FIXES: if (auto x = dyn_cast(y))
+
+  while (auto x = cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:19: warning: cast<> in conditional
+  // CHECK-FIXES: while (auto x = dyn_cast(y))
+
+  if (cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: cast<> in conditional
+  // CHECK-FIXES: if (isa(y))
+
+  while (cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning: cast<> in conditional
+  // CHECK-FIXES: while (isa(y))
+
+  do {
+break;
+  } while (cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: cast<> in conditional
+  // CHECK-FIXES: while (isa(y));
+
+  if (dyn_cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: return value from dyn_cast<> not used [llvm-avoid-cast-in-conditional]
+  // CHECK-FIXES: if (isa(y))
+
+  while (dyn_cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning: return value from dyn_cast<> not used
+  // CHECK-FIXES: while (isa(y))
+
+  do {
+break;
+  } while (dyn_cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: return value from dyn_cast<> not used
+  // CHECK-FIXES: while (isa(y));
+
+  if (z->bar() && isa(z->bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  method '{{.*}}' is called twice and could be expensive [llvm-avoid-cast-in-conditional]
+  // CHECK-FIXES: if (dyn_cast_or_null(z->bar()))
+
+  if (z->bar() && cast(z->bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: method {{'.*'}} is called twice
+  // CHECK-FIXES: if (dyn_cast_or_null(z->bar()))
+
+  if (z->bar() && dyn_cast(z->bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: method {{'.*'}} is called twice
+  // CHECK-FIXES: if (dyn_cast_or_null(z->bar()))
+
+  if (z->bar() && dyn_cast_or_null(z->bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: method {{'.*'}} is called twice
+  // CHECK-FIXES: if (dyn_cast_or_null(z->bar()))
+
+  bool b = z->bar() && cast(z->bar());
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: method {{'.*'}} is called twice
+  // CHECK-FIXES: bool b = dyn_cast_or_null(z->bar());
+
+  // These don't trigger a warning.
+  if (auto x = cast(y)->foo())
+return true;
+  while (auto x = cast(y)->foo())
+break;
+  if (cast(y)->foo())
+return true;
+  while (cast(y)->foo())
+break;
+  if (y && isa(y))
+return true;
+  if (y && cast(z->bar()))
+return true;
+  if (z && cast(y)->foo())
+return true;
+  bool b2 = y && cast(z);
+
+#define CAST(T, Obj) cast(Obj)
+#define AUTO_VAR_CAST(X, Y, Z) auto X = cast(Z)
+#define ISA(T, Obj) isa(Obj)
+#define ISA_OR_NULL(T, Obj) Obj &&isa(Obj)
+
+  if (auto x = CAST(X, y))
+return true;
+  if (AUTO_VAR_CAST(x, X, z))
+return true;
+  if (z->bar() && ISA(Y, z->bar()))
+return true;
+  if (ISA_OR_NULL(Y, z->bar()))
+return true;
+
+  return false;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
@@ -0,0 +1,28 @@
+.. title:: clang-tidy - llvm-avoid-cast-in-conditional
+
+llvm-avoid-cast-in-conditional
+==
+
+  Finds uses of ``cast<>`` in conditionals of ``if``, ``while`` or
+  ``do`` statements, which will assert rather than return a null
+  pointer. It also finds uses of ``dyn_cast<>`` in conditionals where
+  the 

[PATCH] D59746: [LibTooling] Fix '-h' option

[Don Hinton via Phabricator via cfe-commits]

hintonda added a subscriber: arphaman.
hintonda added a comment.

@arphaman, since you added the `-h` option in D37618 
, could you let me know if this change is okay 
with you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:18
+
+AST_MATCHER(Expr, isMacroID) { return Node.getExprLoc().isMacroID(); }
+} // namespace ast_matchers

@aaron.ballman:  This matcher seems genuinely useful.  What do you think about 
moving it to ASTMatchers.h? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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