[PATCH] D55349: Convert some ObjC alloc msgSends to runtime calls

[Pete Cooper via Phabricator via cfe-commits]

pete added a comment.

In D55349#1323926 , @rjmccall wrote:

> LGTM.


Thanks for the review!  Landed in r348687.


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

https://reviews.llvm.org/D55349



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

[libunwind] r348699 - Creating release candidate rc3 from release_701 branch

[Tom Stellard via cfe-commits]

Author: tstellar
Date: Fri Dec  7 21:19:40 2018
New Revision: 348699

URL: http://llvm.org/viewvc/llvm-project?rev=348699&view=rev
Log:
Creating release candidate rc3 from release_701 branch

Added:
libunwind/tags/RELEASE_701/rc3/   (props changed)
  - copied from r348698, libunwind/branches/release_70/

Propchange: libunwind/tags/RELEASE_701/rc3/
--
svn:mergeinfo = /libunwind/trunk:339217


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

r348687 - Convert some ObjC msgSends to runtime calls.

[Pete Cooper via cfe-commits]

Author: pete
Date: Fri Dec  7 21:13:50 2018
New Revision: 348687

URL: http://llvm.org/viewvc/llvm-project?rev=348687&view=rev
Log:
Convert some ObjC msgSends to runtime calls.

It is faster to directly call the ObjC runtime for methods such as 
alloc/allocWithZone instead of sending a message to those functions.

This patch adds support for converting messages to alloc/allocWithZone to their 
equivalent runtime calls.

Tests included for the positive case of applying this transformation, negative 
tests that we ensure we only convert "alloc" to objc_alloc, not "alloc2", and 
also a driver test to ensure we enable this only for supported runtime versions.

Reviewed By: rjmccall

https://reviews.llvm.org/D55349

Added:
cfe/trunk/test/CodeGenObjC/convert-messages-to-runtime-calls.m
cfe/trunk/test/Driver/objc-convert-messages-to-runtime-calls.m
Modified:
cfe/trunk/include/clang/Basic/ObjCRuntime.h
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Frontend/CodeGenOptions.def
cfe/trunk/lib/CodeGen/CGObjC.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/lib/CodeGen/CodeGenModule.h
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp

Modified: cfe/trunk/include/clang/Basic/ObjCRuntime.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/ObjCRuntime.h?rev=348687&r1=348686&r2=348687&view=diff
==
--- cfe/trunk/include/clang/Basic/ObjCRuntime.h (original)
+++ cfe/trunk/include/clang/Basic/ObjCRuntime.h Fri Dec  7 21:13:50 2018
@@ -173,6 +173,43 @@ public:
 llvm_unreachable("bad kind");
   }
 
+  /// Does this runtime provide entrypoints that are likely to be faster
+  /// than an ordinary message send of the "alloc" selector?
+  ///
+  /// The "alloc" entrypoint is guaranteed to be equivalent to just sending the
+  /// corresponding message.  If the entrypoint is implemented naively as just 
a
+  /// message send, using it is a trade-off: it sacrifices a few cycles of
+  /// overhead to save a small amount of code.  However, it's possible for
+  /// runtimes to detect and special-case classes that use "standard"
+  /// alloc behavior; if that's dynamically a large proportion of all
+  /// objects, using the entrypoint will also be faster than using a message
+  /// send.
+  ///
+  /// When this method returns true, Clang will turn non-super message sends of
+  /// certain selectors into calls to the corresponding entrypoint:
+  ///   alloc => objc_alloc
+  ///   allocWithZone:nil => objc_allocWithZone
+  bool shouldUseRuntimeFunctionsForAlloc() const {
+switch (getKind()) {
+case FragileMacOSX:
+  return false;
+case MacOSX:
+  return getVersion() >= VersionTuple(10, 10);
+case iOS:
+  return getVersion() >= VersionTuple(8);
+case WatchOS:
+  return true;
+
+case GCC:
+  return false;
+case GNUstep:
+  return false;
+case ObjFW:
+  return false;
+}
+llvm_unreachable("bad kind");
+  }
+
   /// Does this runtime supports optimized setter entrypoints?
   bool hasOptimizedSetter() const {
 switch (getKind()) {

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=348687&r1=348686&r2=348687&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Fri Dec  7 21:13:50 2018
@@ -1473,6 +1473,10 @@ def fno_zero_initialized_in_bss : Flag<[
 def fobjc_arc : Flag<["-"], "fobjc-arc">, Group, Flags<[CC1Option]>,
   HelpText<"Synthesize retain and release calls for Objective-C pointers">;
 def fno_objc_arc : Flag<["-"], "fno-objc-arc">, Group;
+def fobjc_convert_messages_to_runtime_calls :
+  Flag<["-"], "fobjc-convert-messages-to-runtime-calls">, Group;
+def fno_objc_convert_messages_to_runtime_calls :
+  Flag<["-"], "fno-objc-convert-messages-to-runtime-calls">, Group, 
Flags<[CC1Option]>;
 def fobjc_arc_exceptions : Flag<["-"], "fobjc-arc-exceptions">, 
Group, Flags<[CC1Option]>,
   HelpText<"Use EH-safe code when synthesizing retains and releases in 
-fobjc-arc">;
 def fno_objc_arc_exceptions : Flag<["-"], "fno-objc-arc-exceptions">, 
Group;

Modified: cfe/trunk/include/clang/Frontend/CodeGenOptions.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CodeGenOptions.def?rev=348687&r1=348686&r2=348687&view=diff
==
--- cfe/trunk/include/clang/Frontend/CodeGenOptions.def (original)
+++ cfe/trunk/include/clang/Frontend/CodeGenOptions.def Fri Dec  7 21:13:50 2018
@@ -149,6 +149,8 @@ CODEGENOPT(UniformWGSize , 1, 0) ///
 CODEGENOPT(NoZeroInitializedInBSS , 1, 0) ///< -fno-zero-initialized-in-bss.
 /// Method of Objective-C dispatch to use.
 ENUM_CO

[PATCH] D55470: Cpp11 Braced List Style Ignores BAS_AlwaysBreak

[John Gehrig via Phabricator via cfe-commits]

jgehrig created this revision.
jgehrig added a project: clang.
Herald added a subscriber: cfe-commits.

First patch attempt :)

I've noticed that clang-format does not honor the BAS_AlwaysBreak when 
Cpp11BracedListStyle is set. This seems like a bug, as BAS_DontAlign will 
change the line continuation behavior.

This patch makes BAS_AlwaysBreak affect the line continuation behavior in the 
same way it does with functions or constructors.

Unit Tests should demonstrate the proposed modifications. I also have two 
related patches which I intend to propose: spaces inside Cpp11BracedList {}s, 
and parameterization of the 'magic number' which overrides BinPackArguments for 
Cpp11BracedLists.


Repository:
  rC Clang

https://reviews.llvm.org/D55470

Files:
  lib/Format/ContinuationIndenter.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -7277,6 +7277,60 @@
   verifyFormat("f({}, {{}, {}}, MyMap[{k, v}]);", SpaceBeforeBrace);
 }
 
+TEST_F(FormatTest, Cpp11ListAlwaysBreakOrDontAlign) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+  Style.BinPackArguments = false;
+  Style.ColumnLimit = 40;
+  Style.Cpp11BracedListStyle = true;
+
+  verifyFormat("type foobar_a{\n"
+   "aa,\n"
+   "bb,\n"
+   "cc,\n"
+   "dd,\n"
+   "ee};",
+   Style);
+
+  verifyFormat("type some_namespace::foobar_b{\n"
+   "a, abc, adbdef};",
+   Style);
+
+  verifyFormat("type foobar_c{\n"
+   "an_int,\n"
+   "an_int,\n"
+   "an_int,\n"
+   "an_int,\n"
+   "an_int,\n"
+   "an_int,\n"
+   "an_int,\n"
+   "an_int};",
+   Style);
+
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+
+  verifyFormat("type foobar_a{aa,\n"
+   "bb,\n"
+   "cc,\n"
+   "dd,\n"
+   "ee};",
+   Style);
+
+  verifyFormat("type some_namespace::foobar_b{\n"
+   "a, abc, adbdef};",
+   Style);
+
+  verifyFormat("type foobar_c{an_int,\n"
+   "an_int,\n"
+   "an_int,\n"
+   "an_int,\n"
+   "an_int,\n"
+   "an_int,\n"
+   "an_int,\n"
+   "an_int};",
+   Style);
+}
+
 TEST_F(FormatTest, FormatsBracedListsInColumnLayout) {
   verifyFormat("vector x = {1, 22, 333, , 5, 66, 777,\n"
" 1, 22, 333, , 5, 66, 777,\n"
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -607,7 +607,7 @@
   // disallowing any further line breaks if there is no line break after the
   // opening parenthesis. Don't break if it doesn't conserve columns.
   if (Style.AlignAfterOpenBracket == FormatStyle::BAS_AlwaysBreak &&
-  Previous.isOneOf(tok::l_paren, TT_TemplateOpener, tok::l_square) &&
+  Previous.isOneOf(tok::l_paren, TT_TemplateOpener, tok::l_square, tok::l_brace) &&
   State.Column > getNewLineColumn(State) &&
   (!Previous.Previous || !Previous.Previous->isOneOf(
  tok::kw_for, tok::kw_while, tok::kw_switch)) &&
@@ -625,6 +625,7 @@
 State.Stack.back().NoLineBreak = true;
 
   if (Style.AlignAfterOpenBracket != FormatStyle::BAS_DontAlign &&
+  (!Style.isCpp() || Style.AlignAfterOpenBracket != FormatStyle::BAS_AlwaysBreak) &&
   Previous.opensScope() && Previous.isNot(TT_ObjCMethodExpr) &&
   (Current.isNot(TT_LineComment) || Previous.BlockKind == BK_BracedInit))
 State.Stack.back().Indent = State.Column + Spaces;
@@ -1217,12 +1218,14 @@
 // Indent from 'LastSpace' unless these are fake parentheses encapsulating
 // a builder type call after 'return' or, if the alignment after opening
 // brackets is disabled.
+const bool isAlignParametersMode =
+  Style.AlignAfterOpenBracket != FormatStyle::BAS_DontAlign &&
+  Style.AlignAfterOpenBracket != FormatStyle::BAS_AlwaysBreak;
 if (!Current.isTrailingComment() &&
 (Style.AlignOperands || *I < prec::Assignment) &&
 (!Previous || Previous->isNot(tok::kw_return) ||
  (Style.Language != FormatStyle::LK_Java && *I > 0)) &&
-(Style.AlignAfterOpenBracket != FormatStyle::BAS_DontAlign ||
- *I != prec::Comma || Current.NestingLevel == 0))
+(isAlignParametersMode || *I != prec::Comma || Cur

r348685 - Move diagnostic enums into Basic.

[Richard Trieu via cfe-commits]

Author: rtrieu
Date: Fri Dec  7 21:05:03 2018
New Revision: 348685

URL: http://llvm.org/viewvc/llvm-project?rev=348685&view=rev
Log:
Move diagnostic enums into Basic.

Move enums from */*Diagnostic.h to Basic/Diagnostic*.h.  Basic/AllDiagnostics.h
needs all the enums and moving the sources to Basic prevents a Basic->*->Basic
dependency loop.  This also allows each Basic/Diagnostics*Kinds.td to have a
header at Basic/Diagnostic*.h (except for Common).  The old headers are kept in 
place since other packages are still using them.

Added:
cfe/trunk/include/clang/Basic/DiagnosticAST.h
  - copied, changed from r348541, 
cfe/trunk/include/clang/AST/ASTDiagnostic.h
cfe/trunk/include/clang/Basic/DiagnosticAnalysis.h
  - copied, changed from r348541, 
cfe/trunk/include/clang/Analysis/AnalysisDiagnostic.h
cfe/trunk/include/clang/Basic/DiagnosticComment.h
  - copied, changed from r348541, 
cfe/trunk/include/clang/AST/CommentDiagnostic.h
cfe/trunk/include/clang/Basic/DiagnosticCrossTU.h
  - copied, changed from r348541, 
cfe/trunk/include/clang/CrossTU/CrossTUDiagnostic.h
cfe/trunk/include/clang/Basic/DiagnosticDriver.h
  - copied, changed from r348541, 
cfe/trunk/include/clang/Driver/DriverDiagnostic.h
cfe/trunk/include/clang/Basic/DiagnosticFrontend.h
  - copied, changed from r348541, 
cfe/trunk/include/clang/Frontend/FrontendDiagnostic.h
cfe/trunk/include/clang/Basic/DiagnosticLex.h
  - copied, changed from r348541, 
cfe/trunk/include/clang/Lex/LexDiagnostic.h
cfe/trunk/include/clang/Basic/DiagnosticParse.h
  - copied, changed from r348541, 
cfe/trunk/include/clang/Parse/ParseDiagnostic.h
cfe/trunk/include/clang/Basic/DiagnosticRefactoring.h
  - copied, changed from r348541, 
cfe/trunk/include/clang/Tooling/Refactoring/RefactoringDiagnostic.h
cfe/trunk/include/clang/Basic/DiagnosticSema.h
  - copied, changed from r348541, 
cfe/trunk/include/clang/Sema/SemaDiagnostic.h
cfe/trunk/include/clang/Basic/DiagnosticSerialization.h
  - copied, changed from r348541, 
cfe/trunk/include/clang/Serialization/SerializationDiagnostic.h
Modified:
cfe/trunk/include/clang/AST/ASTDiagnostic.h
cfe/trunk/include/clang/AST/CommentDiagnostic.h
cfe/trunk/include/clang/Analysis/AnalysisDiagnostic.h
cfe/trunk/include/clang/Basic/AllDiagnostics.h
cfe/trunk/include/clang/CrossTU/CrossTUDiagnostic.h
cfe/trunk/include/clang/Driver/DriverDiagnostic.h
cfe/trunk/include/clang/Frontend/FrontendDiagnostic.h
cfe/trunk/include/clang/Lex/LexDiagnostic.h
cfe/trunk/include/clang/Parse/ParseDiagnostic.h
cfe/trunk/include/clang/Sema/SemaDiagnostic.h
cfe/trunk/include/clang/Serialization/SerializationDiagnostic.h
cfe/trunk/include/clang/Tooling/Refactoring/RefactoringDiagnostic.h

Modified: cfe/trunk/include/clang/AST/ASTDiagnostic.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTDiagnostic.h?rev=348685&r1=348684&r2=348685&view=diff
==
--- cfe/trunk/include/clang/AST/ASTDiagnostic.h (original)
+++ cfe/trunk/include/clang/AST/ASTDiagnostic.h Fri Dec  7 21:05:03 2018
@@ -11,19 +11,9 @@
 #define LLVM_CLANG_AST_ASTDIAGNOSTIC_H
 
 #include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticAST.h"
 
 namespace clang {
-  namespace diag {
-enum {
-#define DIAG(ENUM,FLAGS,DEFAULT_MAPPING,DESC,GROUP,\
- SFINAE,NOWERROR,SHOWINSYSHEADER,CATEGORY) ENUM,
-#define ASTSTART
-#include "clang/Basic/DiagnosticASTKinds.inc"
-#undef DIAG
-  NUM_BUILTIN_AST_DIAGNOSTICS
-};
-  }  // end namespace diag
-
   /// DiagnosticsEngine argument formatting function for diagnostics that
   /// involve AST nodes.
   ///

Modified: cfe/trunk/include/clang/AST/CommentDiagnostic.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/CommentDiagnostic.h?rev=348685&r1=348684&r2=348685&view=diff
==
--- cfe/trunk/include/clang/AST/CommentDiagnostic.h (original)
+++ cfe/trunk/include/clang/AST/CommentDiagnostic.h Fri Dec  7 21:05:03 2018
@@ -10,20 +10,7 @@
 #ifndef LLVM_CLANG_AST_COMMENTDIAGNOSTIC_H
 #define LLVM_CLANG_AST_COMMENTDIAGNOSTIC_H
 
-#include "clang/Basic/Diagnostic.h"
-
-namespace clang {
-  namespace diag {
-enum {
-#define DIAG(ENUM,FLAGS,DEFAULT_MAPPING,DESC,GROUP,\
- SFINAE,NOWERROR,SHOWINSYSHEADER,CATEGORY) ENUM,
-#define COMMENTSTART
-#include "clang/Basic/DiagnosticCommentKinds.inc"
-#undef DIAG
-  NUM_BUILTIN_COMMENT_DIAGNOSTICS
-};
-  }  // end namespace diag
-}  // end namespace clang
+#include "clang/Basic/DiagnosticComment.h"
 
 #endif
 

Modified: cfe/trunk/include/clang/Analysis/AnalysisDiagnostic.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/AnalysisDiagnostic.h?rev=348685&r1=348684&r2=348685&view=diff
=

[PATCH] D55468: Use zip_longest for list comparisons.

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur created this revision.
Meinersbur added reviewers: dblaikie, hfinkel.

Use zip_longest in two locations that compare iterator ranges. zip_longest 
allows the iteration using a range-based for-loop and to be symmetric over both 
ranges instead of prioritizing one over the other. In that latter case code 
have to handle the case that the first is longer than the second, the second is 
longer than the first, and both are of the same length, which must partially be 
checked after the loop.

With zip_longest, this becomes an element comparison within the loop like the 
comparison of the elements themselves. The symmetry makes it clearer that 
neither the first and second iterators are handled differently. The iterators 
are not event used directly anymore, just the ranges.


Repository:
  rC Clang

https://reviews.llvm.org/D55468

Files:
  lib/Sema/SemaOverload.cpp
  lib/Serialization/ASTReaderDecl.cpp


Index: lib/Serialization/ASTReaderDecl.cpp
===
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -2913,25 +2913,27 @@
   // Note that pass_object_size attributes are represented in the function's
   // ExtParameterInfo, so we don't need to check them here.
 
-  // Return false if any of the enable_if expressions of A and B are different.
   llvm::FoldingSetNodeID Cand1ID, Cand2ID;
   auto AEnableIfAttrs = A->specific_attrs();
   auto BEnableIfAttrs = B->specific_attrs();
-  auto AEnableIf = AEnableIfAttrs.begin();
-  auto BEnableIf = BEnableIfAttrs.begin();
-  for (; AEnableIf != AEnableIfAttrs.end() && BEnableIf != 
BEnableIfAttrs.end();
-   ++BEnableIf, ++AEnableIf) {
+
+  for (auto Pair : zip_longest(AEnableIfAttrs, BEnableIfAttrs)) {
+// Return false if the number of enable_if attributes is different.
+if (std::get<0>(Pair).hasValue() != std::get<1>(Pair).hasValue())
+  return false;
+
 Cand1ID.clear();
 Cand2ID.clear();
 
-AEnableIf->getCond()->Profile(Cand1ID, A->getASTContext(), true);
-BEnableIf->getCond()->Profile(Cand2ID, B->getASTContext(), true);
+(*std::get<0>(Pair))->getCond()->Profile(Cand1ID, A->getASTContext(), 
true);
+(*std::get<1>(Pair))->getCond()->Profile(Cand2ID, B->getASTContext(), 
true);
+
+// Return false if any of the enable_if expressions of A and B are
+// different.
 if (Cand1ID != Cand2ID)
   return false;
   }
-
-  // Return false if the number of enable_if attributes was different.
-  return AEnableIf == AEnableIfAttrs.end() && BEnableIf == 
BEnableIfAttrs.end();
+  return true;
 }
 
 /// Determine whether the two declarations refer to the same entity.
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -8972,25 +8972,25 @@
   auto Cand1Attrs = Cand1->specific_attrs();
   auto Cand2Attrs = Cand2->specific_attrs();
 
-  auto Cand1I = Cand1Attrs.begin();
   llvm::FoldingSetNodeID Cand1ID, Cand2ID;
-  for (EnableIfAttr *Cand2A : Cand2Attrs) {
-Cand1ID.clear();
-Cand2ID.clear();
-
+  for (auto Pair : zip_longest(Cand1Attrs, Cand2Attrs)) {
 // It's impossible for Cand1 to be better than (or equal to) Cand2 if Cand1
-// has fewer enable_if attributes than Cand2.
-auto Cand1A = Cand1I++;
-if (Cand1A == Cand1Attrs.end())
+// has fewer enable_if attributes than Cand2, and vice versa.
+if (std::get<0>(Pair) == None)
   return Comparison::Worse;
+if (std::get<1>(Pair) == None)
+  return Comparison::Better;
+
+Cand1ID.clear();
+Cand2ID.clear();
 
-Cand1A->getCond()->Profile(Cand1ID, S.getASTContext(), true);
-Cand2A->getCond()->Profile(Cand2ID, S.getASTContext(), true);
+(*std::get<0>(Pair))->getCond()->Profile(Cand1ID, S.getASTContext(), true);
+(*std::get<1>(Pair))->getCond()->Profile(Cand2ID, S.getASTContext(), true);
 if (Cand1ID != Cand2ID)
   return Comparison::Worse;
   }
 
-  return Cand1I == Cand1Attrs.end() ? Comparison::Equal : Comparison::Better;
+  return Comparison::Equal;
 }
 
 static bool isBetterMultiversionCandidate(const OverloadCandidate &Cand1,


Index: lib/Serialization/ASTReaderDecl.cpp
===
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -2913,25 +2913,27 @@
   // Note that pass_object_size attributes are represented in the function's
   // ExtParameterInfo, so we don't need to check them here.
 
-  // Return false if any of the enable_if expressions of A and B are different.
   llvm::FoldingSetNodeID Cand1ID, Cand2ID;
   auto AEnableIfAttrs = A->specific_attrs();
   auto BEnableIfAttrs = B->specific_attrs();
-  auto AEnableIf = AEnableIfAttrs.begin();
-  auto BEnableIf = BEnableIfAttrs.begin();
-  for (; AEnableIf != AEnableIfAttrs.end() && BEnableIf != BEnableIfAttrs.end();
-   ++BEnableIf, ++AEnableIf) {
+
+  for (aut

[PATCH] D55097: [constexpr][c++2a] Try-catch blocks in constexpr functions

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno updated this revision to Diff 177367.
bruno marked an inline comment as done.
bruno added a comment.

Address @aaron.ballman and @erik.pilkington reviews.


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

https://reviews.llvm.org/D55097

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/AST/ExprConstant.cpp
  lib/Sema/SemaDeclCXX.cpp
  test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
  test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
  test/CXX/drs/dr6xx.cpp
  www/cxx_status.html

Index: www/cxx_status.html
===
--- www/cxx_status.html
+++ www/cxx_status.html
@@ -953,13 +953,15 @@
 
   Relaxations of constexpr restrictions
   http://wg21.link/p1064r0";>P1064R0
-  No
+  No
 

 http://wg21.link/p1002r1";>P1002R1
+SVN
   
   
 http://wg21.link/p1327r1";>P1327R1
+No
   
   
 http://wg21.link/p1330r0";>P1330R0
Index: test/CXX/drs/dr6xx.cpp
===
--- test/CXX/drs/dr6xx.cpp
+++ test/CXX/drs/dr6xx.cpp
@@ -492,7 +492,13 @@
   struct C {
 constexpr C(NonLiteral);
 constexpr C(NonLiteral, int) {} // expected-error {{not a literal type}}
-constexpr C() try {} catch (...) {} // expected-error {{function try block}}
+constexpr C() try {} catch (...) {}
+#if __cplusplus <= 201703L
+// expected-error@-2 {{function try block in constexpr constructor is a C++2a extension}}
+#endif
+#if __cplusplus < 201402L
+// expected-error@-5 {{use of this statement in a constexpr constructor is a C++14 extension}}
+#endif
   };
 
   struct D {
Index: test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
===
--- test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
+++ test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
@@ -1,5 +1,6 @@
-// RUN: %clang_cc1 -verify -std=c++11 -fcxx-exceptions -Werror=c++1y-extensions %s
-// RUN: %clang_cc1 -verify -std=c++1y -fcxx-exceptions -DCXX1Y %s
+// RUN: %clang_cc1 -verify -std=c++11 -fcxx-exceptions -Werror=c++1y-extensions -Werror=c++2a-extensions %s
+// RUN: %clang_cc1 -verify -std=c++1y -fcxx-exceptions -DCXX1Y -Werror=c++2a-extensions %s
+// RUN: %clang_cc1 -verify -std=c++2a -fcxx-exceptions -DCXX1Y -DCXX2A %s
 
 namespace N {
   typedef char C;
@@ -49,8 +50,14 @@
 // - its function-body shall not be a function-try-block;
 struct U {
   constexpr U()
-try // expected-error {{function try block not allowed in constexpr constructor}}
+try
+#ifndef CXX2A
+  // expected-error@-2 {{function try block in constexpr constructor is a C++2a extension}}
+#endif
 : u() {
+#ifndef CXX1Y
+  // expected-error@-2 {{use of this statement in a constexpr constructor is a C++14 extension}}
+#endif
   } catch (...) {
 throw;
   }
Index: test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
===
--- test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
+++ test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
@@ -1,5 +1,6 @@
-// RUN: %clang_cc1 -verify -fcxx-exceptions -triple=x86_64-linux-gnu -std=c++11 -Werror=c++1y-extensions %s
-// RUN: %clang_cc1 -verify -fcxx-exceptions -triple=x86_64-linux-gnu -std=c++1y -DCXX1Y %s
+// RUN: %clang_cc1 -verify -fcxx-exceptions -triple=x86_64-linux-gnu -std=c++11 -Werror=c++1y-extensions -Werror=c++2a-extensions %s
+// RUN: %clang_cc1 -verify -fcxx-exceptions -triple=x86_64-linux-gnu -std=c++1y -DCXX1Y -Werror=c++2a-extensions %s
+// RUN: %clang_cc1 -verify -fcxx-exceptions -triple=x86_64-linux-gnu -std=c++2a -DCXX1Y -DCXX2A %s
 
 namespace N {
   typedef char C;
@@ -78,7 +79,12 @@
 };
 struct T3 {
   constexpr T3 &operator=(const T3&) const = default;
-  // expected-error@-1 {{an explicitly-defaulted copy assignment operator may not have 'const' or 'volatile' qualifiers}}
+#ifndef CXX2A
+  // expected-error@-2 {{an explicitly-defaulted copy assignment operator may not have 'const' or 'volatile' qualifiers}}
+#else
+  // expected-warning@-4 {{explicitly defaulted copy assignment operator is implicitly deleted}}
+  // expected-note@-5 {{function is implicitly deleted because its declared type does not match the type of an implicit copy assignment operator}}
+#endif
 };
 #endif
 struct U {
@@ -129,9 +135,22 @@
 x:
   return 0;
 }
+constexpr int DisallowedStmtsCXX1Y_2_1() {
+  try {
+return 0;
+  } catch (...) {
+  merp: goto merp; // expected-error {{statement not allowed in constexpr function}}
+  }
+}
 constexpr int DisallowedStmtsCXX1Y_3() {
   //  - a try-block,
-  try {} catch (...) {} // expected-error {{statement not allowed in constexpr function}}
+  try {} catch (...) {}
+#ifndef CXX2A
+  // expected-error@-2 {{use of this statement in a constexpr function is a C++2a extension}}
+#ifndef CXX1Y
+  // expected-error@-4 {{use of this statement in a constexpr function is a C++14 extension}}
+#endif
+#endif
  

[PATCH] D55465: Stop tracking retain count of OSObject after escape to void * / other primitive types

[George Karpenkov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC348675: Stop tracking retain count of OSObject after escape 
to void * / other primitive… (authored by george.karpenkov, committed by ).
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D55465?vs=177361&id=177362#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D55465

Files:
  lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
  test/Analysis/osobject-retain-release.cpp


Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
+++ lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
@@ -574,6 +574,25 @@
   return State;
 }
 
+static bool isPointerToObject(QualType QT) {
+  QualType PT = QT->getPointeeType();
+  if (!PT.isNull())
+if (PT->getAsCXXRecordDecl())
+  return true;
+  return false;
+}
+
+/// Whether the tracked value should be escaped on a given call.
+/// OSObjects are escaped when passed to void * / etc.
+static bool shouldEscapeArgumentOnCall(const CallEvent &CE, unsigned ArgIdx,
+   const RefVal *TrackedValue) {
+  if (TrackedValue->getObjKind() != RetEffect::OS)
+return false;
+  if (ArgIdx >= CE.parameters().size())
+return false;
+  return !isPointerToObject(CE.parameters()[ArgIdx]->getType());
+}
+
 void RetainCountChecker::checkSummary(const RetainSummary &Summ,
   const CallEvent &CallOrMsg,
   CheckerContext &C) const {
@@ -592,6 +611,10 @@
   state = updateOutParameter(state, V, Effect);
 } else if (SymbolRef Sym = V.getAsLocSymbol()) {
   if (const RefVal *T = getRefBinding(state, Sym)) {
+
+if (shouldEscapeArgumentOnCall(CallOrMsg, idx, T))
+  Effect = StopTrackingHard;
+
 state = updateSymbol(state, Sym, *T, Effect, hasErr, C);
 if (hasErr) {
   ErrorRange = CallOrMsg.getArgSourceRange(idx);
Index: test/Analysis/osobject-retain-release.cpp
===
--- test/Analysis/osobject-retain-release.cpp
+++ test/Analysis/osobject-retain-release.cpp
@@ -89,6 +89,13 @@
   static OSObject *safeMetaCast(const OSObject *inst, const OSMetaClass *meta);
 };
 
+void escape(void *);
+
+void test_escaping_into_voidstar() {
+  OSObject *obj = new OSObject;
+  escape(obj);
+}
+
 void test_no_infinite_check_recursion(MyArray *arr) {
   OSObject *input = new OSObject;
   OSObject *o = arr->generateObject(input);


Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
+++ lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
@@ -574,6 +574,25 @@
   return State;
 }
 
+static bool isPointerToObject(QualType QT) {
+  QualType PT = QT->getPointeeType();
+  if (!PT.isNull())
+if (PT->getAsCXXRecordDecl())
+  return true;
+  return false;
+}
+
+/// Whether the tracked value should be escaped on a given call.
+/// OSObjects are escaped when passed to void * / etc.
+static bool shouldEscapeArgumentOnCall(const CallEvent &CE, unsigned ArgIdx,
+   const RefVal *TrackedValue) {
+  if (TrackedValue->getObjKind() != RetEffect::OS)
+return false;
+  if (ArgIdx >= CE.parameters().size())
+return false;
+  return !isPointerToObject(CE.parameters()[ArgIdx]->getType());
+}
+
 void RetainCountChecker::checkSummary(const RetainSummary &Summ,
   const CallEvent &CallOrMsg,
   CheckerContext &C) const {
@@ -592,6 +611,10 @@
   state = updateOutParameter(state, V, Effect);
 } else if (SymbolRef Sym = V.getAsLocSymbol()) {
   if (const RefVal *T = getRefBinding(state, Sym)) {
+
+if (shouldEscapeArgumentOnCall(CallOrMsg, idx, T))
+  Effect = StopTrackingHard;
+
 state = updateSymbol(state, Sym, *T, Effect, hasErr, C);
 if (hasErr) {
   ErrorRange = CallOrMsg.getArgSourceRange(idx);
Index: test/Analysis/osobject-retain-release.cpp
===
--- test/Analysis/osobject-retain-release.cpp
+++ test/Analysis/osobject-retain-release.cpp
@@ -89,6 +89,13 @@
   static OSObject *safeMetaCast(const OSObject *inst, const OSMetaClass *meta);
 };
 
+void escape(void *);
+
+void test_escaping_into_voidstar() {
+  OSObject *obj = new OSObject;
+  escape(obj);
+}
+
 void test_no_infinite_check_recursion(MyArray *arr) {
   OSObject *input = new OSObject;
   OSObject *o = arr->generateObject(input);

r348675 - Stop tracking retain count of OSObject after escape to void * / other primitive types

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Fri Dec  7 17:18:40 2018
New Revision: 348675

URL: http://llvm.org/viewvc/llvm-project?rev=348675&view=rev
Log:
Stop tracking retain count of OSObject after escape to void * / other primitive 
types

Escaping to void * / uint64_t / others non-OSObject * should stop tracking,
as such functions can have heterogeneous semantics depending on context,
and can not always be annotated.

rdar://46439133

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

Modified:

cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
cfe/trunk/test/Analysis/osobject-retain-release.cpp

Modified: 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp?rev=348675&r1=348674&r2=348675&view=diff
==
--- 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp 
(original)
+++ 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp 
Fri Dec  7 17:18:40 2018
@@ -574,6 +574,25 @@ static ProgramStateRef updateOutParamete
   return State;
 }
 
+static bool isPointerToObject(QualType QT) {
+  QualType PT = QT->getPointeeType();
+  if (!PT.isNull())
+if (PT->getAsCXXRecordDecl())
+  return true;
+  return false;
+}
+
+/// Whether the tracked value should be escaped on a given call.
+/// OSObjects are escaped when passed to void * / etc.
+static bool shouldEscapeArgumentOnCall(const CallEvent &CE, unsigned ArgIdx,
+   const RefVal *TrackedValue) {
+  if (TrackedValue->getObjKind() != RetEffect::OS)
+return false;
+  if (ArgIdx >= CE.parameters().size())
+return false;
+  return !isPointerToObject(CE.parameters()[ArgIdx]->getType());
+}
+
 void RetainCountChecker::checkSummary(const RetainSummary &Summ,
   const CallEvent &CallOrMsg,
   CheckerContext &C) const {
@@ -592,6 +611,10 @@ void RetainCountChecker::checkSummary(co
   state = updateOutParameter(state, V, Effect);
 } else if (SymbolRef Sym = V.getAsLocSymbol()) {
   if (const RefVal *T = getRefBinding(state, Sym)) {
+
+if (shouldEscapeArgumentOnCall(CallOrMsg, idx, T))
+  Effect = StopTrackingHard;
+
 state = updateSymbol(state, Sym, *T, Effect, hasErr, C);
 if (hasErr) {
   ErrorRange = CallOrMsg.getArgSourceRange(idx);

Modified: cfe/trunk/test/Analysis/osobject-retain-release.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/osobject-retain-release.cpp?rev=348675&r1=348674&r2=348675&view=diff
==
--- cfe/trunk/test/Analysis/osobject-retain-release.cpp (original)
+++ cfe/trunk/test/Analysis/osobject-retain-release.cpp Fri Dec  7 17:18:40 2018
@@ -89,6 +89,13 @@ struct OSMetaClassBase {
   static OSObject *safeMetaCast(const OSObject *inst, const OSMetaClass *meta);
 };
 
+void escape(void *);
+
+void test_escaping_into_voidstar() {
+  OSObject *obj = new OSObject;
+  escape(obj);
+}
+
 void test_no_infinite_check_recursion(MyArray *arr) {
   OSObject *input = new OSObject;
   OSObject *o = arr->generateObject(input);


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

[PATCH] D55456: [CUDA] added missing 'inline' for the functions defined in the header.

[Artem Belevich via Phabricator via cfe-commits]

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

jlebar@ LGTM'ed via email.
Landed in rL348662 


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

https://reviews.llvm.org/D55456



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

[PATCH] D55455: Remove stat cache chaining as it's no longer needed after PTH support has been removed

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

In D55455#1323836 , @dexonsmith wrote:

> This patch LGTM, but before committing I suggest sending an RFC to cfe-dev 
> and waiting a few days.


I posted to `cfe-dev` to check if there are any objections.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55455



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

[PATCH] D55097: [constexpr][c++2a] Try-catch blocks in constexpr functions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Some minor nits from the peanut gallery.




Comment at: lib/AST/ExprConstant.cpp:4278
+  case Stmt::CXXTryStmtClass:
+// Evaluate try blocks by evaluating all sub statements
+return EvaluateStmt(Result, Info, cast(S)->getTryBlock(), 
Case);

Missing full stop.



Comment at: lib/Sema/SemaDeclCXX.cpp:1906
+  Cxx2aLoc = S->getBeginLoc();
+for (Stmt *SubStmt : S->children())
+  if (SubStmt &&

I'd appreciate curly braces here, even though they're not strictly required by 
the coding standard. ;-)



Comment at: lib/Sema/SemaDeclCXX.cpp:1971
+  SourceLocation Cxx1yLoc, Cxx2aLoc;
+  for (Stmt *SubStmt : Body->children())
+if (SubStmt &&

Likewise here.


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

https://reviews.llvm.org/D55097



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

[PATCH] D55097: [constexpr][c++2a] Try-catch blocks in constexpr functions

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added a comment.

LGTM, but you should probably let @rsmith have the final word!




Comment at: lib/Sema/SemaDeclCXX.cpp:1916-1919
+for (Stmt *SubStmt : S->children())
+  if (SubStmt &&
+  !CheckConstexprFunctionStmt(SemaRef, Dcl, SubStmt, ReturnStmts,
+  Cxx1yLoc, Cxx2aLoc))

Might be clearer to just write `if (!CheckConstexprFunctionStmt(SemaRef, Dcl, 
cast(SubStmt)->getHandlerBlock()))`, rather than looping over 1 
statement. Could you also add that example I posted as a testcase for this?


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

https://reviews.llvm.org/D55097



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

[PATCH] D55463: Introduce a source minimizer that reduces source to directives that might affect the dependency list for a compilation

[Alex Lorenz via Phabricator via cfe-commits]

arphaman created this revision.
arphaman added reviewers: dexonsmith, Bigcheese, dblaikie, vsapsai, sammccall, 
rsmith, bruno.
Herald added subscribers: jkorous, mgorny.

This patch introduces a dependency directives source minimizer to clang that 
minimizes header and source files to the minimum necessary preprocessor 
directives for evaluating includes. It reduces the source down to `#define`, 
`#include`, `#import`, `@import`, and any conditional preprocessor logic that 
contains one of those.

The source minimizer works by lexing the input with a custom fast lexer that 
recognizes the preprocessor directives it cares about, and emitting those 
directives in the minimized source. It ignores source code, comments, and 
normalizes whitespace. It gives up and fails if seems any directives that it 
doesn't recognize as valid (e.g. `#define 0`).

In addition to the source minimizer this patch adds a 
`print-dependency-directives-minimized-source` CC1 option that allows you to 
invoke the minimizer using clang directly.

There a couple of known issues with the source minimizer:

- It fails to detect `@import` that was formed in a macro expansion. We are 
planning to add a warning to discourage this use.
- It fails to detect `_Pragma ("clang import")`. We are planning to probably 
add a warning to discourage this use.
- It assumes raw string literals are valid when minimizing source without 
respecting language mode.

This is based on code that was included in the original WIP patch I posted 
before the dev meeting: https://reviews.llvm.org/D53354 . It's based on the 
original filter-to-includes code written by @dexonsmith . We are planning to 
use to implement fast dependency scanning for explicit module builds.


Repository:
  rC Clang

https://reviews.llvm.org/D55463

Files:
  include/clang/Basic/DiagnosticFrontendKinds.td
  include/clang/Driver/CC1Options.td
  include/clang/Frontend/FrontendActions.h
  include/clang/Frontend/FrontendOptions.h
  include/clang/Lex/DependencyDirectivesSourceMinimizer.h
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/FrontendActions.cpp
  lib/FrontendTool/ExecuteCompilerInvocation.cpp
  lib/Lex/CMakeLists.txt
  lib/Lex/DependencyDirectivesSourceMinimizer.cpp
  test/Frontend/minimize_source_to_dependency_directives.c
  test/Frontend/minimize_source_to_dependency_directives_error.c
  unittests/Lex/CMakeLists.txt
  unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp

Index: unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
===
--- /dev/null
+++ unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
@@ -0,0 +1,463 @@
+//===- unittests/Lex/DependencyDirectivesSourceMinimizer.cpp -  ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/Lex/DependencyDirectivesSourceMinimizer.h"
+#include "llvm/ADT/SmallString.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace clang;
+using namespace clang::minimize_source_to_dependency_directives;
+
+namespace clang {
+
+bool minimizeSourceToDependencyDirectives(StringRef Input,
+  SmallVectorImpl &Out) {
+  SmallVector Tokens;
+  return minimizeSourceToDependencyDirectives(Input, Out, Tokens);
+}
+
+} // end namespace clang
+
+namespace {
+
+TEST(MinimizeSourceToDependencyDirectivesTest, Empty) {
+  SmallVector Out;
+  SmallVector Tokens;
+
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives("", Out, Tokens));
+  EXPECT_TRUE(Out.empty());
+  ASSERT_EQ(1u, Tokens.size());
+  ASSERT_EQ(pp_eof, Tokens.back().K);
+
+  ASSERT_FALSE(
+  minimizeSourceToDependencyDirectives("abc def\nxyz", Out, Tokens));
+  EXPECT_TRUE(Out.empty());
+  ASSERT_EQ(1u, Tokens.size());
+  ASSERT_EQ(pp_eof, Tokens.back().K);
+}
+
+TEST(MinimizeSourceToDependencyDirectivesTest, AllTokens) {
+  SmallVector Out;
+  SmallVector Tokens;
+
+  ASSERT_FALSE(
+  minimizeSourceToDependencyDirectives("#define A\n"
+   "#undef A\n"
+   "#endif\n"
+   "#if A\n"
+   "#ifdef A\n"
+   "#ifndef A\n"
+   "#elif A\n"
+   "#else\n"
+   "#include \n"
+   "#include_next \n"
+   "#__include_macros \n"
+   "#import \n"
+   "@import A;\n"
+   "#pragma clang module import A\n",
+ 

[clang-tools-extra] r348666 - [Documentation] Alphabetical order in new checks list.

[Eugene Zelenko via cfe-commits]

Author: eugenezelenko
Date: Fri Dec  7 16:07:34 2018
New Revision: 348666

URL: http://llvm.org/viewvc/llvm-project?rev=348666&view=rev
Log:
[Documentation] Alphabetical order in new checks list.

Modified:
clang-tools-extra/trunk/docs/ReleaseNotes.rst

Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=348666&r1=348665&r2=348666&view=diff
==
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Fri Dec  7 16:07:34 2018
@@ -122,13 +122,6 @@ Improvements to clang-tidy
   Flags uses of ``absl::StrCat()`` to append to a ``std::string``. Suggests
   ``absl::StrAppend()`` should be used instead.
 
-- New :doc:`bugprone-too-small-loop-variable
-  ` check.
-
-  Detects those ``for`` loops that have a loop variable with a "too small" type
-  which means this type can't represent all values which are part of the
-  iteration range.
-
 - New :doc:`abseil-upgrade-duration-conversions
   ` check.
 
@@ -136,6 +129,13 @@ Improvements to clang-tidy
   argument needs an explicit cast to continue compiling after upcoming API
   changes.
 
+- New :doc:`bugprone-too-small-loop-variable
+  ` check.
+
+  Detects those ``for`` loops that have a loop variable with a "too small" type
+  which means this type can't represent all values which are part of the
+  iteration range.
+
 - New :doc:`cppcoreguidelines-macro-usage
   ` check.
 


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

[PATCH] D55451: [tests] Fix the FileManagerTest getVirtualFile test on Windows

[Stella Stamenova via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rC348665: [tests] Fix the FileManagerTest getVirtualFile test 
on Windows (authored by stella.stamenova, committed by ).

Repository:
  rC Clang

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

https://reviews.llvm.org/D55451

Files:
  unittests/Basic/FileManagerTest.cpp


Index: unittests/Basic/FileManagerTest.cpp
===
--- unittests/Basic/FileManagerTest.cpp
+++ unittests/Basic/FileManagerTest.cpp
@@ -351,22 +351,34 @@
 
 // getVirtualFile should always fill the real path.
 TEST_F(FileManagerTest, getVirtualFileFillsRealPathName) {
+  SmallString<64> CustomWorkingDir;
+#ifdef _WIN32
+  CustomWorkingDir = "C:/";
+#else
+  CustomWorkingDir = "/";
+#endif
+
+  auto FS = IntrusiveRefCntPtr(
+  new llvm::vfs::InMemoryFileSystem);
+  // setCurrentworkingdirectory must finish without error.
+  ASSERT_TRUE(!FS->setCurrentWorkingDirectory(CustomWorkingDir));
+
+  FileSystemOptions Opts;
+  FileManager Manager(Opts, FS);
+
   // Inject fake files into the file system.
   auto statCache = llvm::make_unique();
   statCache->InjectDirectory("/tmp", 42);
   statCache->InjectFile("/tmp/test", 43);
-  manager.addStatCache(std::move(statCache));
+
+  Manager.addStatCache(std::move(statCache));
 
   // Check for real path.
-  const FileEntry *file = manager.getVirtualFile("/tmp/test", 123, 1);
+  const FileEntry *file = Manager.getVirtualFile("/tmp/test", 123, 1);
   ASSERT_TRUE(file != nullptr);
   ASSERT_TRUE(file->isValid());
-  SmallString<64> ExpectedResult;
-#ifdef _WIN32
-  ExpectedResult = "C:/";
-#else
-  ExpectedResult = "/";
-#endif
+  SmallString<64> ExpectedResult = CustomWorkingDir;
+
   llvm::sys::path::append(ExpectedResult, "tmp", "test");
   EXPECT_EQ(file->tryGetRealPathName(), ExpectedResult);
 }


Index: unittests/Basic/FileManagerTest.cpp
===
--- unittests/Basic/FileManagerTest.cpp
+++ unittests/Basic/FileManagerTest.cpp
@@ -351,22 +351,34 @@
 
 // getVirtualFile should always fill the real path.
 TEST_F(FileManagerTest, getVirtualFileFillsRealPathName) {
+  SmallString<64> CustomWorkingDir;
+#ifdef _WIN32
+  CustomWorkingDir = "C:/";
+#else
+  CustomWorkingDir = "/";
+#endif
+
+  auto FS = IntrusiveRefCntPtr(
+  new llvm::vfs::InMemoryFileSystem);
+  // setCurrentworkingdirectory must finish without error.
+  ASSERT_TRUE(!FS->setCurrentWorkingDirectory(CustomWorkingDir));
+
+  FileSystemOptions Opts;
+  FileManager Manager(Opts, FS);
+
   // Inject fake files into the file system.
   auto statCache = llvm::make_unique();
   statCache->InjectDirectory("/tmp", 42);
   statCache->InjectFile("/tmp/test", 43);
-  manager.addStatCache(std::move(statCache));
+
+  Manager.addStatCache(std::move(statCache));
 
   // Check for real path.
-  const FileEntry *file = manager.getVirtualFile("/tmp/test", 123, 1);
+  const FileEntry *file = Manager.getVirtualFile("/tmp/test", 123, 1);
   ASSERT_TRUE(file != nullptr);
   ASSERT_TRUE(file->isValid());
-  SmallString<64> ExpectedResult;
-#ifdef _WIN32
-  ExpectedResult = "C:/";
-#else
-  ExpectedResult = "/";
-#endif
+  SmallString<64> ExpectedResult = CustomWorkingDir;
+
   llvm::sys::path::append(ExpectedResult, "tmp", "test");
   EXPECT_EQ(file->tryGetRealPathName(), ExpectedResult);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r348665 - [tests] Fix the FileManagerTest getVirtualFile test on Windows

[Stella Stamenova via cfe-commits]

Author: stella.stamenova
Date: Fri Dec  7 15:50:05 2018
New Revision: 348665

URL: http://llvm.org/viewvc/llvm-project?rev=348665&view=rev
Log:
[tests] Fix the FileManagerTest getVirtualFile test on Windows

Summary: The test passes on Windows only when it is executed on the C: drive. 
If the build and tests run on a different drive, the test is currently failing.

Reviewers: kadircet, asmith

Subscribers: cfe-commits

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

Modified:
cfe/trunk/unittests/Basic/FileManagerTest.cpp

Modified: cfe/trunk/unittests/Basic/FileManagerTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/FileManagerTest.cpp?rev=348665&r1=348664&r2=348665&view=diff
==
--- cfe/trunk/unittests/Basic/FileManagerTest.cpp (original)
+++ cfe/trunk/unittests/Basic/FileManagerTest.cpp Fri Dec  7 15:50:05 2018
@@ -351,22 +351,34 @@ TEST_F(FileManagerTest, makeAbsoluteUses
 
 // getVirtualFile should always fill the real path.
 TEST_F(FileManagerTest, getVirtualFileFillsRealPathName) {
+  SmallString<64> CustomWorkingDir;
+#ifdef _WIN32
+  CustomWorkingDir = "C:/";
+#else
+  CustomWorkingDir = "/";
+#endif
+
+  auto FS = IntrusiveRefCntPtr(
+  new llvm::vfs::InMemoryFileSystem);
+  // setCurrentworkingdirectory must finish without error.
+  ASSERT_TRUE(!FS->setCurrentWorkingDirectory(CustomWorkingDir));
+
+  FileSystemOptions Opts;
+  FileManager Manager(Opts, FS);
+
   // Inject fake files into the file system.
   auto statCache = llvm::make_unique();
   statCache->InjectDirectory("/tmp", 42);
   statCache->InjectFile("/tmp/test", 43);
-  manager.addStatCache(std::move(statCache));
+
+  Manager.addStatCache(std::move(statCache));
 
   // Check for real path.
-  const FileEntry *file = manager.getVirtualFile("/tmp/test", 123, 1);
+  const FileEntry *file = Manager.getVirtualFile("/tmp/test", 123, 1);
   ASSERT_TRUE(file != nullptr);
   ASSERT_TRUE(file->isValid());
-  SmallString<64> ExpectedResult;
-#ifdef _WIN32
-  ExpectedResult = "C:/";
-#else
-  ExpectedResult = "/";
-#endif
+  SmallString<64> ExpectedResult = CustomWorkingDir;
+
   llvm::sys::path::append(ExpectedResult, "tmp", "test");
   EXPECT_EQ(file->tryGetRealPathName(), ExpectedResult);
 }


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

[PATCH] D54604: Automatic variable initialization

[JF Bastien via Phabricator via cfe-commits]

jfb updated this revision to Diff 177341.
jfb added a comment.

- Fix typo


Repository:
  rC Clang

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

https://reviews.llvm.org/D54604

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Basic/LangOptions.def
  include/clang/Basic/LangOptions.h
  include/clang/Driver/Options.td
  include/clang/Driver/ToolChain.h
  lib/CodeGen/CGDecl.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGenCXX/auto-var-init.cpp
  test/CodeGenCXX/trivial-auto-var-init-attribute.cpp
  test/CodeGenCXX/trivial-auto-var-init.cpp
  test/Driver/clang_f_opts.c
  test/Misc/warning-flags.c
  test/Sema/attr-trivial_auto_init.c
  test/Sema/uninit-variables.c

Index: test/Sema/uninit-variables.c
===
--- test/Sema/uninit-variables.c
+++ test/Sema/uninit-variables.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -Wuninitialized -Wconditional-uninitialized -fsyntax-only -fblocks %s -verify
+// RUN: %clang_cc1 -fsyntax-only -Wuninitialized -Wconditional-uninitialized -ftrivial-auto-var-init=pattern -fsyntax-only -fblocks %s -verify
 
 typedef __typeof(sizeof(int)) size_t;
 void *malloc(size_t);
Index: test/Sema/attr-trivial_auto_init.c
===
--- /dev/null
+++ test/Sema/attr-trivial_auto_init.c
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only
+
+void good() {
+int dont_initialize_me __attribute((trivial_auto_init("uninitialized")));
+int zero_me __attribute((trivial_auto_init("zero")));
+int pattern_me __attribute((trivial_auto_init("pattern")));
+}
+
+void bad() {
+int im_bad __attribute((trivial_auto_init)); // expected-error {{'trivial_auto_init' attribute takes one argument}}
+int im_baaad __attribute((trivial_auto_init("uninitialized", "zero"))); // expected-error {{'trivial_auto_init' attribute takes one argument}}
+static int come_on __attribute((trivial_auto_init("uninitialized"))); // expected-warning {{'trivial_auto_init' attribute only applies to local variables}}
+int you_know __attribute((trivial_auto_init("pony"))); // expected-warning {{'trivial_auto_init' attribute argument not supported: pony}}
+int and_the_whole_world_has_to __attribute((trivial_auto_init(uninitialized))); // expected-error {{'trivial_auto_init' attribute requires a string}}
+}
+
+extern int answer_right_now __attribute((trivial_auto_init("uninitialized"))); // expected-warning {{'trivial_auto_init' attribute only applies to local variables}}
+int just_to_tell_you_once_again __attribute((trivial_auto_init("uninitialized"))); // expected-warning {{'trivial_auto_init' attribute only applies to local variables}}
+static int whos_bad __attribute((trivial_auto_init("uninitialized"))); // expected-warning {{'trivial_auto_init' attribute only applies to local variables}}
+
+void the_word_is_out() __attribute((trivial_auto_init("uninitialized"))) {} // expected-warning {{'trivial_auto_init' attribute only applies to local variables}}
+void youre_doin_wrong(__attribute((trivial_auto_init("uninitialized"))) int a) {} // expected-warning {{'trivial_auto_init' attribute only applies to local variables}}
+
+struct GonnaLockYouUp {
+  __attribute((trivial_auto_init("uninitialized"))) int before_too_long; // expected-warning {{'trivial_auto_init' attribute only applies to local variables}}
+} __attribute((trivial_auto_init("uninitialized"))); // expected-warning {{'trivial_auto_init' attribute only applies to local variables}}
Index: test/Misc/warning-flags.c
===
--- test/Misc/warning-flags.c
+++ test/Misc/warning-flags.c
@@ -18,7 +18,7 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (75):
+CHECK: Warnings without flags (76):
 CHECK-NEXT:   ext_excess_initializers
 CHECK-NEXT:   ext_excess_initializers_in_char_array_initializer
 CHECK-NEXT:   ext_expected_semi_decl_list
@@ -53,6 +53,7 @@
 CHECK-NEXT:   warn_drv_assuming_mfloat_abi_is
 CHECK-NEXT:   warn_drv_clang_unsupported
 CHECK-NEXT:   warn_drv_pch_not_first_include
+CHECK-NEXT:   warn_drv_trivial_auto_var_init_zero_disabled
 CHECK-NEXT:   warn_dup_category_def
 CHECK-NEXT:   warn_enum_value_overflow
 CHECK-NEXT:   warn_expected_qualified_after_typename
Index: test/Driver/clang_f_opts.c
===
--- test/Driver/clang_f_opts.c
+++ test/Driver/clang_f_opts.c
@@ -542,3 +542,12 @@
 // RUN: %clang -### -S -fomit-frame-pointer -fno-omit-frame-pointer -pg %s 2>&1 | FileCheck -check-prefix=CHECK-MIX-NO-OMIT-FP-PG %s
 // CHECK-NO-MIX-OMIT-FP-PG: '-fomit-frame-pointer' not allowed with '-pg'
 // CHECK-MIX-NO-OMIT-FP-PG-NOT: '-fomit-frame-pointer' not allowed with

[PATCH] D55443: [test] Capture stderr from 'tar --version' call as well

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski resigned from this revision.
krytarowski added a comment.
Herald added a subscriber: krytarowski.

I don't feel enough comfortable with this Python code.


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

https://reviews.llvm.org/D55443



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

[PATCH] D54604: Automatic variable initialization

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added inline comments.



Comment at: include/clang/Basic/DiagnosticDriverKinds.td:412
+  "Enable it at your own peril for benchmarking purpose only with "
+  "-enable-trivial-auto-var-init-zero-knowning-it-will-be-removed-from-clang">;
+

s/knowning/knowing/ gives 
`-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang`


Repository:
  rC Clang

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

https://reviews.llvm.org/D54604



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

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 177335.

Repository:
  rC Clang

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

https://reviews.llvm.org/D51329

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeLoc.h
  clang/include/clang/AST/TypeNodes.def
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/ARCMigrate/TransGCAttrs.cpp
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTDiagnostic.cpp
  clang/lib/AST/ASTStructuralEquivalence.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypeLoc.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/Sema/address_space_print_macro.c
  clang/test/Sema/address_spaces.c
  clang/test/SemaCUDA/qualifiers.cu
  clang/tools/libclang/CIndex.cpp

Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -1618,6 +1618,10 @@
   return Visit(TL.getInnerLoc());
 }
 
+bool CursorVisitor::VisitMacroDefinedTypeLoc(MacroDefinedTypeLoc TL) {
+  return Visit(TL.getInnerLoc());
+}
+
 bool CursorVisitor::VisitPointerTypeLoc(PointerTypeLoc TL) {
   return Visit(TL.getPointeeLoc());
 }
Index: clang/test/SemaCUDA/qualifiers.cu
===
--- clang/test/SemaCUDA/qualifiers.cu
+++ clang/test/SemaCUDA/qualifiers.cu
@@ -20,16 +20,16 @@
 #if defined(__CUDA_ARCH__)
 // NVPTX does not support TLS
 __device__ int __thread device_tls_var; // expected-error {{thread-local storage is not supported for the current target}}
-// CHECK-DEVICE: device_tls_var 'int' tls
+// CHECK-DEVICE: device_tls_var 'int':'int' tls
 __shared__ int __thread shared_tls_var; // expected-error {{thread-local storage is not supported for the current target}}
-// CHECK-DEVICE: shared_tls_var 'int' tls
+// CHECK-DEVICE: shared_tls_var 'int':'int' tls
 #else
 // Device-side vars should not produce any errors during host-side
 // compilation.
 __device__ int __thread device_tls_var;
-// CHECK-HOST: device_tls_var 'int' tls
+// CHECK-HOST: device_tls_var 'int':'int' tls
 __shared__ int __thread shared_tls_var;
-// CHECK-HOST: shared_tls_var 'int' tls
+// CHECK-HOST: shared_tls_var 'int':'int' tls
 #endif
 
 __global__ void g1(int x) {}
Index: clang/test/Sema/address_spaces.c
===
--- clang/test/Sema/address_spaces.c
+++ clang/test/Sema/address_spaces.c
@@ -71,7 +71,7 @@
 
 // Clang extension doesn't forbid operations on pointers to different address spaces.
 char* cmp(_AS1 char *x,  _AS2 char *y) {
-  return x < y ? x : y; // expected-error{{conditional operator with the second and third operands of type  ('__attribute__((address_space(1))) char *' and '__attribute__((address_space(2))) char *') which are pointers to non-overlapping address spaces}}
+  return x < y ? x : y; // expected-error{{conditional operator with the second and third operands of type  ('_AS1 char *' and '_AS2 char *') which are pointers to non-overlapping address spaces}}
 }
 
 struct SomeStruct {
Index: clang/test/Sema/address_space_print_macro.c
===
--- /dev/null
+++ clang/test/Sema/address_space_print_macro.c
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify
+
+#define AS1 __attribute__((address_space(1)))
+#define AS2 __attribute__((address_space(2), annotate("foo")))
+
+#define AS(i) address_space(i)
+#define AS3 __attribute__((AS(3)))
+
+#define ATTR __attribute__
+#define AS4 ATTR((AS(4)))
+#define AS5 __attribute__((address_space(5))) char
+
+char *cmp(AS1 char *x, AS2 char *y) {
+  return x < y ? x : y; // expected-error{{conditional operator with the second and third operands of type  ('AS1 char *' and 'AS2 char *') which are pointers to non-overlapping address spaces}}
+}
+
+__attribute__((address_space(1))) char test_array[10];
+void test3(void) {
+  extern void test3_helper(char *p); // expected-note{{passing argument to parameter 'p' here}}
+  test3_helper(test_array);  // expected-error{{passing '__attribute__((address_space(1))) char *' to parameter of type 'char *' changes address space of pointer}}
+}
+
+char AS2 *test4_array;
+void test4(void) {
+  extern void test3_helper(char *p); // expected-note{{passing argument to parameter 'p' here}}
+  test3_helper(test4_array); // expected-error{{passing 'AS2 char *' to parameter of type 'char *' changes address space of pointer}

[PATCH] D55429: [analyzer] Add CheckerManager::getChecker, make sure that a registry function registers no more than 1 checker

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

Szelethus added a comment.

In D55429#1324035 , @NoQ wrote:

> Aha, ok, so what's the final procedure




In D55429#1324035 , @NoQ wrote:

> Aha, ok, so what's the final procedure now to register a checker that's 
> artificially split in two? Something like this, right?
>
>   def CommonModel : Checker<"Common">,
> HelpText<"Doesn't emit warnings but models common stuff.">;
>  
>   def SubChecker : Checker<"Sub">,
> HelpText<"Emits common warnings for the sub-stuff.">,
> Dependencies<[CommonModel]>;
>
>
>
>
>   void registerCommonModel(CheckerManager &Mgr) {
> Mgr.registerChecker();
>   }
>  
>   void registerSubChecker(CheckerManager &Mgr) {
> CommonModel *Model = Mgr.getChecker();
> Model->EnableSubChecker = true;
>   }
>
>
> This looks quite usable to me.


Correct! But since I spent so much time with these files, and I don't expect 
maaajor changes to them in the foreseeable future, I'll take the time to 
properly document how the frontend of the analyzer (especially how checker 
registration) works. Maybe with the new sphinx format if it goes through by 
then, but any format is better than none.


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

https://reviews.llvm.org/D55429



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

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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Also you might need to rebase on top of D55400 
.


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

https://reviews.llvm.org/D54438



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

[PATCH] D54604: Automatic variable initialization

[JF Bastien via Phabricator via cfe-commits]

jfb updated this revision to Diff 177331.
jfb added a comment.

- Update warning-flags.c


Repository:
  rC Clang

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

https://reviews.llvm.org/D54604

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Basic/LangOptions.def
  include/clang/Basic/LangOptions.h
  include/clang/Driver/Options.td
  include/clang/Driver/ToolChain.h
  lib/CodeGen/CGDecl.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGenCXX/auto-var-init.cpp
  test/CodeGenCXX/trivial-auto-var-init-attribute.cpp
  test/CodeGenCXX/trivial-auto-var-init.cpp
  test/Driver/clang_f_opts.c
  test/Misc/warning-flags.c
  test/Sema/attr-trivial_auto_init.c
  test/Sema/uninit-variables.c

Index: test/Sema/uninit-variables.c
===
--- test/Sema/uninit-variables.c
+++ test/Sema/uninit-variables.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -Wuninitialized -Wconditional-uninitialized -fsyntax-only -fblocks %s -verify
+// RUN: %clang_cc1 -fsyntax-only -Wuninitialized -Wconditional-uninitialized -ftrivial-auto-var-init=pattern -fsyntax-only -fblocks %s -verify
 
 typedef __typeof(sizeof(int)) size_t;
 void *malloc(size_t);
Index: test/Sema/attr-trivial_auto_init.c
===
--- /dev/null
+++ test/Sema/attr-trivial_auto_init.c
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only
+
+void good() {
+int dont_initialize_me __attribute((trivial_auto_init("uninitialized")));
+int zero_me __attribute((trivial_auto_init("zero")));
+int pattern_me __attribute((trivial_auto_init("pattern")));
+}
+
+void bad() {
+int im_bad __attribute((trivial_auto_init)); // expected-error {{'trivial_auto_init' attribute takes one argument}}
+int im_baaad __attribute((trivial_auto_init("uninitialized", "zero"))); // expected-error {{'trivial_auto_init' attribute takes one argument}}
+static int come_on __attribute((trivial_auto_init("uninitialized"))); // expected-warning {{'trivial_auto_init' attribute only applies to local variables}}
+int you_know __attribute((trivial_auto_init("pony"))); // expected-warning {{'trivial_auto_init' attribute argument not supported: pony}}
+int and_the_whole_world_has_to __attribute((trivial_auto_init(uninitialized))); // expected-error {{'trivial_auto_init' attribute requires a string}}
+}
+
+extern int answer_right_now __attribute((trivial_auto_init("uninitialized"))); // expected-warning {{'trivial_auto_init' attribute only applies to local variables}}
+int just_to_tell_you_once_again __attribute((trivial_auto_init("uninitialized"))); // expected-warning {{'trivial_auto_init' attribute only applies to local variables}}
+static int whos_bad __attribute((trivial_auto_init("uninitialized"))); // expected-warning {{'trivial_auto_init' attribute only applies to local variables}}
+
+void the_word_is_out() __attribute((trivial_auto_init("uninitialized"))) {} // expected-warning {{'trivial_auto_init' attribute only applies to local variables}}
+void youre_doin_wrong(__attribute((trivial_auto_init("uninitialized"))) int a) {} // expected-warning {{'trivial_auto_init' attribute only applies to local variables}}
+
+struct GonnaLockYouUp {
+  __attribute((trivial_auto_init("uninitialized"))) int before_too_long; // expected-warning {{'trivial_auto_init' attribute only applies to local variables}}
+} __attribute((trivial_auto_init("uninitialized"))); // expected-warning {{'trivial_auto_init' attribute only applies to local variables}}
Index: test/Misc/warning-flags.c
===
--- test/Misc/warning-flags.c
+++ test/Misc/warning-flags.c
@@ -18,7 +18,7 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (75):
+CHECK: Warnings without flags (76):
 CHECK-NEXT:   ext_excess_initializers
 CHECK-NEXT:   ext_excess_initializers_in_char_array_initializer
 CHECK-NEXT:   ext_expected_semi_decl_list
@@ -53,6 +53,7 @@
 CHECK-NEXT:   warn_drv_assuming_mfloat_abi_is
 CHECK-NEXT:   warn_drv_clang_unsupported
 CHECK-NEXT:   warn_drv_pch_not_first_include
+CHECK-NEXT:   warn_drv_trivial_auto_var_init_zero_disabled
 CHECK-NEXT:   warn_dup_category_def
 CHECK-NEXT:   warn_enum_value_overflow
 CHECK-NEXT:   warn_expected_qualified_after_typename
Index: test/Driver/clang_f_opts.c
===
--- test/Driver/clang_f_opts.c
+++ test/Driver/clang_f_opts.c
@@ -542,3 +542,12 @@
 // RUN: %clang -### -S -fomit-frame-pointer -fno-omit-frame-pointer -pg %s 2>&1 | FileCheck -check-prefix=CHECK-MIX-NO-OMIT-FP-PG %s
 // CHECK-NO-MIX-OMIT-FP-PG: '-fomit-frame-pointer' not allowed with '-pg'
 // CHECK-MIX-NO-OMIT-FP-PG-NOT: '-fomit-frame-pointer' no

[PATCH] D55388: [analyzer] MoveChecker Pt.8: Add checks for dereferencing a smart pointer after move.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 177332.
NoQ added a comment.

Fix typo.


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

https://reviews.llvm.org/D55388

Files:
  lib/StaticAnalyzer/Checkers/MoveChecker.cpp
  test/Analysis/use-after-move.cpp

Index: test/Analysis/use-after-move.cpp
===
--- test/Analysis/use-after-move.cpp
+++ test/Analysis/use-after-move.cpp
@@ -1,20 +1,26 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.Move -verify %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
-// RUN:  -analyzer-config exploration_strategy=unexplored_first_queue
+// RUN:  -analyzer-config exploration_strategy=unexplored_first_queue\
+// RUN:  -analyzer-checker debug.ExprInspection
 // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.Move -verify %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
-// RUN:  -analyzer-config exploration_strategy=dfs -DDFS=1
+// RUN:  -analyzer-config exploration_strategy=dfs -DDFS=1\
+// RUN:  -analyzer-checker debug.ExprInspection
 // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.Move -verify %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
 // RUN:  -analyzer-config exploration_strategy=unexplored_first_queue\
-// RUN:  -analyzer-config alpha.cplusplus.Move:Aggressive=true -DAGGRESSIVE
+// RUN:  -analyzer-config alpha.cplusplus.Move:Aggressive=true -DAGGRESSIVE\
+// RUN:  -analyzer-checker debug.ExprInspection
 // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.Move -verify %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
 // RUN:  -analyzer-config exploration_strategy=dfs -DDFS=1\
-// RUN:  -analyzer-config alpha.cplusplus.Move:Aggressive=true -DAGGRESSIVE
+// RUN:  -analyzer-config alpha.cplusplus.Move:Aggressive=true -DAGGRESSIVE\
+// RUN:  -analyzer-checker debug.ExprInspection
 
 #include "Inputs/system-header-simulator-cxx.h"
 
+void clang_analyzer_warnIfReached();
+
 class B {
 public:
   B() = default;
@@ -810,7 +816,19 @@
 // expected-note@-4{{Object 'P' is moved}}
 // expected-note@-4{{Method called on moved-from object 'P'}}
 #endif
-*P += 1; // FIXME: Should warn that the pointer is null.
+
+// Because that well-defined state is null, dereference is still UB.
+// Note that in aggressive mode we already warned about 'P',
+// so no extra warning is generated.
+*P += 1;
+#ifndef AGGRESSIVE
+// expected-warning@-2{{Dereference of null smart pointer 'P' of type 'std::unique_ptr'}}
+// expected-note@-14{{Smart pointer 'P' of type 'std::unique_ptr' is reset to null when moved from}}
+// expected-note@-4{{Dereference of null smart pointer 'P' of type 'std::unique_ptr'}}
+#endif
+
+// The program should have crashed by now.
+clang_analyzer_warnIfReached(); // no-warning
   }
 };
 
@@ -827,3 +845,9 @@
   P.get(); // expected-warning{{Method called on moved-from object 'P'}}
// expected-note@-1{{Method called on moved-from object 'P'}}
 }
+
+void localUniquePtrWithArrow(std::unique_ptr P) {
+  std::unique_ptr Q = std::move(P); // expected-note{{Smart pointer 'P' of type 'std::unique_ptr' is reset to null when moved from}}
+  P->foo(); // expected-warning{{Dereference of null smart pointer 'P' of type 'std::unique_ptr'}}
+// expected-note@-1{{Dereference of null smart pointer 'P' of type 'std::unique_ptr'}}
+}
Index: lib/StaticAnalyzer/Checkers/MoveChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MoveChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MoveChecker.cpp
@@ -61,19 +61,35 @@
   const char *NL, const char *Sep) const override;
 
 private:
-  enum MisuseKind { MK_FunCall, MK_Copy, MK_Move };
+  enum MisuseKind { MK_FunCall, MK_Copy, MK_Move, MK_Dereference };
+  enum StdObjectKind { SK_NonStd, SK_Unsafe, SK_Safe, SK_SmartPtr };
+
+  static bool misuseCausesCrash(MisuseKind MK) {
+return MK == MK_Dereference;
+  }
 
   struct ObjectKind {
-bool Local : 1; // Is this a local variable or a local rvalue reference?
-bool STL : 1; // Is this an object of a standard type?
+// Is this a local variable or a local rvalue reference?
+bool IsLocal : 1;
+// Is this an STL object? If so, of what kind?
+StdObjectKind StdKind : 2;
+  };
+
+  // STL smart pointers are automatically re-initialized to null when moved
+  // from. So we can't warn on many methods, but we can warn when it is
+  // dereferenced, which is UB even if the resulting lvalue never gets read.
+  const llvm::StringSet<> StdSmartPtrClasses = {
+  "shared_ptr",
+  "unique_ptr",
+  "weak_ptr",
   };
 
   // Not all of these are entirely move-safe, but they do provide *some*
   // guarantees, and it means that somebody is using them after move
   // in a valid manner.
-  //

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 177330.
MyDeveloperDay added a comment.

Minor alterations to Release notes and Documentation to address review comments


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

https://reviews.llvm.org/D55433

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/UseNodiscardCheck.cpp
  clang-tidy/modernize/UseNodiscardCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-use-nodiscard.rst
  test/clang-tidy/modernize-use-nodiscard.cpp

Index: test/clang-tidy/modernize-use-nodiscard.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-nodiscard.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s modernize-use-nodiscard %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-nodiscard.ReplacementString, value: 'NO_DISCARD'}]}" \
+// RUN: -- -std=c++17 \
+
+// TODO using macros to avoid CHECK-FIXES not being able to find [[xxx]]
+#define MUST_USE_RESULT __attribute__((warn_unused_result))
+#define NO_DISCARD [[nodiscard]]
+#define NO_RETURN [[noreturn]]
+
+class Foo
+{
+public:
+bool f1() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f1' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f1() const;
+
+bool f2(int) const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f2' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f2(int) const;
+
+bool f3(const int &) const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f3' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f3(const int &) const;
+
+bool f4(void) const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f4' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f4(void) const;
+
+// negative tests
+
+void f5() const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: void f5() const;
+
+bool f6();
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool f6();
+
+bool f7(int &);
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool f7(int &);
+
+bool f8(int &) const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool f8(int &) const;
+
+bool f9(int *) const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool f9(int *) const;
+
+bool f10(const int &,int &) const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool f10(const int &,int &) const;
+
+NO_DISCARD bool f12() const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: NO_DISCARD bool f12() const;
+
+MUST_USE_RESULT bool f13() const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: MUST_USE_RESULT bool f13() const;
+   
+// TODO fix CHECK-FIXES to handle "[[" "]]"
+[[nodiscard]] bool f11() const;
+// CHECK-MESSAGES-NOT: warning:
+// _CHECK-FIXES: [[nodiscard]] bool f11() const;
+
+bool _f20() const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool _f20() const;
+
+NO_RETURN bool f21() const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: NO_RETURN bool f21() const;
+
+~Foo();
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: ~Foo();
+
+bool operator +=(int) const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool operator +=(int) const;
+   
+// extra keywords (virtual,inline,const) on return type
+
+virtual bool f14() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f14' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD virtual bool f14() const;
+
+const bool f15() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f15' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD const bool f15() const;
+
+inline const bool f16() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f16' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD inline const bool f16() const;
+
+inline virtual const bool f17() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f17' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD inline virtual const bool f17() const;
+
+// inline with body
+bool f18() const {
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f18' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f18() const {
+return true;
+}
+
+bool f19() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f19' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f19() const;
+};
+
+bool Foo::f19() const
+// CHECK-MESSA

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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Generally, because the primary user of checker names is clang-tidy, the correct 
name to display is the name of the specific checker that the user needs to 
disable in order to get rid of the warning.


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

https://reviews.llvm.org/D54438



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

[PATCH] D54604: Automatic variable initialization

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

I added an option that's required when using `clang` directly (*not* `-cc1`!) 
If you use `-ftrivial-auto-var-init=zero` without 
`-enable-trivial-auto-var-init-zero-knowning-it-will-be-removed-from-clang` in 
`clang` it'll generate a warning and change initialization to pattern 
initialization instead. That's roughly along the lines people were going for on 
cfe-dev.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54604



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

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

[Artem Dergachev via Phabricator via cfe-commits]

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

I'm seeing no problems with this patch and i'm happy that we're replacing hacks 
with well-defined patterns :)

In D54438#1322970 , @Szelethus wrote:

> - Register the checker after it's dependencies (accidentally left this change 
> in the next branch). This implies that `unix.MallocChecker:Optimistic` is no 
> longer a thing, and is now called `unix.DynamicMemoryModeling:Optimistic`. 
> This //could// break backward compatibility, but is also addressable, since 
> it can always be converted to whatever full name `DynamicMemoryModeling` will 
> have in the future. @george.karpenkov @NoQ feelings on this?


I believe nobody uses this flag, feel free to remove it entirely.

In D54438#1322970 , @Szelethus wrote:

> - In `test/Analysis/Inputs/expected-plists/nullability-notes.m.plist`, the 
> name of the checker associated with a report changed, but this is **NOT** an 
> issue raised by this patch -- simply changing the order of 
> `analyzer-checker=ABC,CBA` to `CBA,ABC` will cause the same issue. Please 
> don't mind me, but I'm not immediately interested in fixing this issue.


It wasn't correct in the first place (should have been 
`NullablePassedToNonnull` instead). Anyway, no problem!


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

https://reviews.llvm.org/D54438



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

[PATCH] D54604: Automatic variable initialization

[JF Bastien via Phabricator via cfe-commits]

jfb updated this revision to Diff 177328.
jfb added a comment.

- Add an ugly option to enable zero init


Repository:
  rC Clang

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

https://reviews.llvm.org/D54604

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Basic/LangOptions.def
  include/clang/Basic/LangOptions.h
  include/clang/Driver/Options.td
  include/clang/Driver/ToolChain.h
  lib/CodeGen/CGDecl.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGenCXX/auto-var-init.cpp
  test/CodeGenCXX/trivial-auto-var-init-attribute.cpp
  test/CodeGenCXX/trivial-auto-var-init.cpp
  test/Driver/clang_f_opts.c
  test/Sema/attr-trivial_auto_init.c
  test/Sema/uninit-variables.c

Index: test/Sema/uninit-variables.c
===
--- test/Sema/uninit-variables.c
+++ test/Sema/uninit-variables.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -Wuninitialized -Wconditional-uninitialized -fsyntax-only -fblocks %s -verify
+// RUN: %clang_cc1 -fsyntax-only -Wuninitialized -Wconditional-uninitialized -ftrivial-auto-var-init=pattern -fsyntax-only -fblocks %s -verify
 
 typedef __typeof(sizeof(int)) size_t;
 void *malloc(size_t);
Index: test/Sema/attr-trivial_auto_init.c
===
--- /dev/null
+++ test/Sema/attr-trivial_auto_init.c
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only
+
+void good() {
+int dont_initialize_me __attribute((trivial_auto_init("uninitialized")));
+int zero_me __attribute((trivial_auto_init("zero")));
+int pattern_me __attribute((trivial_auto_init("pattern")));
+}
+
+void bad() {
+int im_bad __attribute((trivial_auto_init)); // expected-error {{'trivial_auto_init' attribute takes one argument}}
+int im_baaad __attribute((trivial_auto_init("uninitialized", "zero"))); // expected-error {{'trivial_auto_init' attribute takes one argument}}
+static int come_on __attribute((trivial_auto_init("uninitialized"))); // expected-warning {{'trivial_auto_init' attribute only applies to local variables}}
+int you_know __attribute((trivial_auto_init("pony"))); // expected-warning {{'trivial_auto_init' attribute argument not supported: pony}}
+int and_the_whole_world_has_to __attribute((trivial_auto_init(uninitialized))); // expected-error {{'trivial_auto_init' attribute requires a string}}
+}
+
+extern int answer_right_now __attribute((trivial_auto_init("uninitialized"))); // expected-warning {{'trivial_auto_init' attribute only applies to local variables}}
+int just_to_tell_you_once_again __attribute((trivial_auto_init("uninitialized"))); // expected-warning {{'trivial_auto_init' attribute only applies to local variables}}
+static int whos_bad __attribute((trivial_auto_init("uninitialized"))); // expected-warning {{'trivial_auto_init' attribute only applies to local variables}}
+
+void the_word_is_out() __attribute((trivial_auto_init("uninitialized"))) {} // expected-warning {{'trivial_auto_init' attribute only applies to local variables}}
+void youre_doin_wrong(__attribute((trivial_auto_init("uninitialized"))) int a) {} // expected-warning {{'trivial_auto_init' attribute only applies to local variables}}
+
+struct GonnaLockYouUp {
+  __attribute((trivial_auto_init("uninitialized"))) int before_too_long; // expected-warning {{'trivial_auto_init' attribute only applies to local variables}}
+} __attribute((trivial_auto_init("uninitialized"))); // expected-warning {{'trivial_auto_init' attribute only applies to local variables}}
Index: test/Driver/clang_f_opts.c
===
--- test/Driver/clang_f_opts.c
+++ test/Driver/clang_f_opts.c
@@ -542,3 +542,12 @@
 // RUN: %clang -### -S -fomit-frame-pointer -fno-omit-frame-pointer -pg %s 2>&1 | FileCheck -check-prefix=CHECK-MIX-NO-OMIT-FP-PG %s
 // CHECK-NO-MIX-OMIT-FP-PG: '-fomit-frame-pointer' not allowed with '-pg'
 // CHECK-MIX-NO-OMIT-FP-PG-NOT: '-fomit-frame-pointer' not allowed with '-pg'
+
+// RUN: %clang -### -S -ftrivial-auto-var-init=uninitialized %s 2>&1 | FileCheck -check-prefix=CHECK-TRIVIAL-UNINIT %s
+// RUN: %clang -### -S -ftrivial-auto-var-init=pattern %s 2>&1 | FileCheck -check-prefix=CHECK-TRIVIAL-PATTERN %s
+// RUN: %clang -### -S -ftrivial-auto-var-init=zero -enable-trivial-auto-var-init-zero-knowning-it-will-be-removed-from-clang %s 2>&1 | FileCheck -check-prefix=CHECK-TRIVIAL-ZERO-GOOD %s
+// RUN: %clang -### -S -ftrivial-auto-var-init=zero %s 2>&1 | FileCheck -check-prefix=CHECK-TRIVIAL-ZERO-BAD %s
+// CHECK-TRIVIAL-UNINIT-NOT: hasn't been enabled
+// CHECK-TRIVIAL-PATTERN-NOT: hasn't been enabled
+// CHECK-TRIVIAL-ZERO-GOOD-NOT: hasn't been enabled
+// CHECK-TRIVIAL-ZERO-BAD: hasn't been enabled
Index: test/CodeGenCXX/trivial-auto-var-init.cpp
===

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 177327.
MyDeveloperDay added a comment.

- Move the final conditional statements into AST_MATCHERS
- Add additional IgnoreInternalFunctions option to allow adding of 
``[[nodiscard]]`` to functions starting with _ (e.g. _Foo())


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

https://reviews.llvm.org/D55433

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/UseNodiscardCheck.cpp
  clang-tidy/modernize/UseNodiscardCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-use-nodiscard.rst
  test/clang-tidy/modernize-use-nodiscard.cpp

Index: test/clang-tidy/modernize-use-nodiscard.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-nodiscard.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s modernize-use-nodiscard %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-nodiscard.ReplacementString, value: 'NO_DISCARD'}]}" \
+// RUN: -- -std=c++17 \
+
+// TODO using macros to avoid CHECK-FIXES not being able to find [[xxx]]
+#define MUST_USE_RESULT __attribute__((warn_unused_result))
+#define NO_DISCARD [[nodiscard]]
+#define NO_RETURN [[noreturn]]
+
+class Foo
+{
+public:
+bool f1() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f1' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f1() const;
+
+bool f2(int) const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f2' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f2(int) const;
+
+bool f3(const int &) const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f3' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f3(const int &) const;
+
+bool f4(void) const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f4' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f4(void) const;
+
+// negative tests
+
+void f5() const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: void f5() const;
+
+bool f6();
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool f6();
+
+bool f7(int &);
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool f7(int &);
+
+bool f8(int &) const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool f8(int &) const;
+
+bool f9(int *) const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool f9(int *) const;
+
+bool f10(const int &,int &) const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool f10(const int &,int &) const;
+
+NO_DISCARD bool f12() const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: NO_DISCARD bool f12() const;
+
+MUST_USE_RESULT bool f13() const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: MUST_USE_RESULT bool f13() const;
+   
+// TODO fix CHECK-FIXES to handle "[[" "]]"
+[[nodiscard]] bool f11() const;
+// CHECK-MESSAGES-NOT: warning:
+// _CHECK-FIXES: [[nodiscard]] bool f11() const;
+
+bool _f20() const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool _f20() const;
+
+NO_RETURN bool f21() const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: NO_RETURN bool f21() const;
+
+~Foo();
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: ~Foo();
+
+bool operator +=(int) const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool operator +=(int) const;
+   
+// extra keywords (virtual,inline,const) on return type
+
+virtual bool f14() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f14' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD virtual bool f14() const;
+
+const bool f15() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f15' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD const bool f15() const;
+
+inline const bool f16() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f16' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD inline const bool f16() const;
+
+inline virtual const bool f17() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f17' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD inline virtual const bool f17() const;
+
+// inline with body
+bool f18() const {
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f18' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f18() const {
+return true;
+}
+
+bool f19() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f19' should be marked NO_DISCARD [modernize-use-no

[PATCH] D55429: [analyzer] Add CheckerManager::getChecker, make sure that a registry function registers no more than 1 checker

[Artem Dergachev via Phabricator via cfe-commits]

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

Aha, ok, so what's the final procedure now to register a checker that's 
artificially split in two? Something like this, right?

  def CommonModel : Checker<"Common">,
HelpText<"Doesn't emit warnings but models common stuff.">;
  
  def SubChecker : Checker<"Sub">,
HelpText<"Emits common warnings for the sub-stuff.">,
Dependencies<[CommonModel]>;



  void registerCommonModel(CheckerManager &Mgr) {
Mgr.registerChecker();
  }
  
  void registerSubChecker(CheckerManager &Mgr) {
CommonModel *Model = Mgr.getChecker();
Model->EnableSubChecker = true;
  }

This looks quite usable to me.




Comment at: test/Analysis/free.c:1
-// RUN: %clang_analyze_cc1 -analyzer-store=region 
-analyzer-checker=core,unix.Malloc -fblocks -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-store=region 
-analyzer-checker=core,unix.Malloc -fblocks -verify -analyzer-config 
unix.Malloc:Optimistic=true %s
+// RUN: %clang_analyze_cc1 -fblocks -verify %s -analyzer-store=region \
+// RUN:   -analyzer-checker=core \

When cleaning this stuff up, please feel free to drop `-analyzer-store=region` 
entirely (same for other options that allow only one value).


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

https://reviews.llvm.org/D55429



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

[PATCH] D55245: [clang-tidy] Add the abseil-duration-subtraction check

[Hyrum Wright via Phabricator via cfe-commits]

hwright updated this revision to Diff 177325.
hwright marked 7 inline comments as done.
hwright added a comment.

Use an `IndexedMap` instead of an `std::unordered_map`


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

https://reviews.llvm.org/D55245

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/DurationComparisonCheck.cpp
  clang-tidy/abseil/DurationRewriter.cpp
  clang-tidy/abseil/DurationRewriter.h
  clang-tidy/abseil/DurationSubtractionCheck.cpp
  clang-tidy/abseil/DurationSubtractionCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-duration-subtraction.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/Inputs/absl/time/time.h
  test/clang-tidy/abseil-duration-comparison.cpp
  test/clang-tidy/abseil-duration-factory-float.cpp
  test/clang-tidy/abseil-duration-factory-scale.cpp
  test/clang-tidy/abseil-duration-subtraction.cpp

Index: test/clang-tidy/abseil-duration-subtraction.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-duration-subtraction.cpp
@@ -0,0 +1,56 @@
+// RUN: %check_clang_tidy %s abseil-duration-subtraction %t -- -- -I %S/Inputs
+
+#include "absl/time/time.h"
+
+void f() {
+  double x;
+  absl::Duration d, d1;
+
+  x = absl::ToDoubleSeconds(d) - 1.0;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleSeconds(d - absl::Seconds(1))
+  x = absl::ToDoubleSeconds(d) - absl::ToDoubleSeconds(d1);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleSeconds(d - d1);
+  x = absl::ToDoubleSeconds(d) - 6.5 - 8.0;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleSeconds(d - absl::Seconds(6.5)) - 8.0;
+  x = absl::ToDoubleHours(d) - 1.0;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleHours(d - absl::Hours(1))
+  x = absl::ToDoubleMinutes(d) - 1;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleMinutes(d - absl::Minutes(1))
+  x = absl::ToDoubleMilliseconds(d) - 9;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleMilliseconds(d - absl::Milliseconds(9))
+  x = absl::ToDoubleMicroseconds(d) - 9;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleMicroseconds(d - absl::Microseconds(9))
+  x = absl::ToDoubleNanoseconds(d) - 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleNanoseconds(d - absl::Nanoseconds(42))
+
+  // We can rewrite the argument of the duration conversion
+#define THIRTY absl::Seconds(30)
+  x = absl::ToDoubleSeconds(THIRTY) - 1.0;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleSeconds(THIRTY - absl::Seconds(1))
+#undef THIRTY
+
+  // Some other contexts
+  if (absl::ToDoubleSeconds(d) - 1.0 > 10) {}
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: if (absl::ToDoubleSeconds(d - absl::Seconds(1)) > 10) {}
+
+  // These should not match
+  x = 5 - 6;
+  x = 4 - absl::ToDoubleSeconds(d) - 6.5 - 8.0;
+  x = absl::ToDoubleSeconds(d) + 1.0;
+  x = absl::ToDoubleSeconds(d) * 1.0;
+  x = absl::ToDoubleSeconds(d) / 1.0;
+
+#define MINUS_FIVE(z) absl::ToDoubleSeconds(z) - 5
+  x = MINUS_FIVE(d);
+#undef MINUS_FIVE
+}
Index: test/clang-tidy/abseil-duration-factory-scale.cpp
===
--- test/clang-tidy/abseil-duration-factory-scale.cpp
+++ test/clang-tidy/abseil-duration-factory-scale.cpp
@@ -1,32 +1,6 @@
-// RUN: %check_clang_tidy %s abseil-duration-factory-scale %t
+// RUN: %check_clang_tidy %s abseil-duration-factory-scale %t -- -- -I %S/Inputs
 
-// Mimic the implementation of absl::Duration
-namespace absl {
-
-class Duration {};
-
-Duration Nanoseconds(long long);
-Duration Microseconds(long long);
-Duration Milliseconds(long long);
-Duration Seconds(long long);
-Duration Minutes(long long);
-Duration Hours(long long);
-
-#define GENERATE_DURATION_FACTORY_OVERLOADS(NAME) \
-  Duration NAME(float n); \
-  Duration NAME(double n);\
-  template\
-  Duration NAME(T n);
-
-GENERATE_DURATION_FACTORY_OVERLOADS(Nano

[PATCH] D55245: [clang-tidy] Add the abseil-duration-subtraction check

[Hyrum Wright via Phabricator via cfe-commits]

hwright added inline comments.



Comment at: clang-tidy/abseil/DurationRewriter.cpp:35
+getInverseForScale(DurationScale Scale) {
+  static const std::unordered_map>

lebedev.ri wrote:
> hwright wrote:
> > lebedev.ri wrote:
> > > https://llvm.org/docs/ProgrammersManual.html#other-map-like-container-options
> > > > We never use hash_set and unordered_set because they are generally very 
> > > > expensive (each insertion requires a malloc) and very non-portable.
> > > 
> > > Since the key is an enum, [[ 
> > > https://llvm.org/docs/ProgrammersManual.html#llvm-adt-indexedmap-h | 
> > > `llvm/ADT/IndexedMap.h` ]] should be a much better fit.
> > It doesn't look like `IndexedMap` has a constructor which takes an 
> > initializer list.  Without it, this code get a bit more difficult to read, 
> > and I'd prefer to optimize for readability here.
> The manual still 'recommends' not to use them.
> Simple solution: immediately invoked lambda
> Better solution: try to add such constructor to `IndexedMap`.
In hopes of not making this too much of a yak shave, I've gone with the 
immediately invoked lambda.



Comment at: clang-tidy/abseil/DurationRewriter.cpp:74
+ Node, *Result.Context))) {
+return tooling::fixit::getText(*MaybeCallArg, *Result.Context).str();
+  }

lebedev.ri wrote:
> hwright wrote:
> > lebedev.ri wrote:
> > > So you generate fix-it, and then immediately degrade it into a string. 
> > > Weird.
> > This doesn't generate a fix-it, it just fetches the text of the given node 
> > as a `StringRef` but we're returning a `string`, so we need to convert.
> > 
> > Is there a more canonical method I should use to fetch a node's text?
> I don't know the answer, but have you tried looking what 
> `tooling::fixit::getText()` does internally?
I have.  It calls `internal::getText`, which, from the namespace, I'm hesitant 
to call in this context.



Comment at: clang-tidy/abseil/DurationRewriter.cpp:156
+llvm::Optional getScaleForInverse(llvm::StringRef Name) {
+  static const llvm::DenseMap ScaleMap(
+  {{"ToDoubleHours", DurationScale::Hours},

lebedev.ri wrote:
> Are you very sure this shouldn't be [[ 
> https://llvm.org/docs/ProgrammersManual.html#llvm-adt-stringmap-h | 
> `StringMap` ]]?
Nope.  Thanks for the catch!


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

https://reviews.llvm.org/D55245



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

[PATCH] D52117: Generate llvm.loop.parallel_accesses instead of llvm.mem.parallel_loop_access metadata.

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur updated this revision to Diff 177324.
Meinersbur added a comment.

- Fix wrong patch upload
- Simplify access group emission ... .. possible due to the added possibility 
for instructions to belong to multiple access groups in D52116 
. However, the number of access groups is not 
minimized anymore.


Repository:
  rC Clang

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

https://reviews.llvm.org/D52117

Files:
  lib/CodeGen/CGLoopInfo.cpp
  lib/CodeGen/CGLoopInfo.h
  test/CodeGenCXX/pragma-loop-safety-imperfectly_nested.cpp
  test/CodeGenCXX/pragma-loop-safety-nested.cpp
  test/CodeGenCXX/pragma-loop-safety-outer.cpp
  test/CodeGenCXX/pragma-loop-safety.cpp
  test/OpenMP/for_codegen.cpp
  test/OpenMP/for_simd_codegen.cpp
  test/OpenMP/loops_explicit_clauses_codegen.cpp
  test/OpenMP/ordered_codegen.cpp
  test/OpenMP/parallel_for_simd_codegen.cpp
  test/OpenMP/schedule_codegen.cpp
  test/OpenMP/simd_codegen.cpp
  test/OpenMP/simd_metadata.c
  test/OpenMP/target_parallel_for_simd_codegen.cpp
  test/OpenMP/target_simd_codegen.cpp
  test/OpenMP/taskloop_simd_codegen.cpp

Index: test/OpenMP/taskloop_simd_codegen.cpp
===
--- test/OpenMP/taskloop_simd_codegen.cpp
+++ test/OpenMP/taskloop_simd_codegen.cpp
@@ -83,17 +83,17 @@
 // CHECK: [[LB_I32:%.+]] = trunc i64 [[LB_VAL]] to i32
 // CHECK: store i32 [[LB_I32]], i32* [[CNT:%.+]],
 // CHECK: br label
-// CHECK: [[VAL:%.+]] = load i32, i32* [[CNT]],{{.*}}!llvm.mem.parallel_loop_access [[LOOP1:!.+]]
+// CHECK: [[VAL:%.+]] = load i32, i32* [[CNT]],{{.*}}!llvm.access.group
 // CHECK: [[VAL_I64:%.+]] = sext i32 [[VAL]] to i64
-// CHECK: [[UB_VAL:%.+]] = load i64, i64* [[UB]],{{.*}}!llvm.mem.parallel_loop_access [[LOOP1]]
+// CHECK: [[UB_VAL:%.+]] = load i64, i64* [[UB]],{{.*}}!llvm.access.group
 // CHECK: [[CMP:%.+]] = icmp ule i64 [[VAL_I64]], [[UB_VAL]]
 // CHECK: br i1 [[CMP]], label %{{.+}}, label %{{.+}}
-// CHECK: load i32, i32* %{{.*}}!llvm.mem.parallel_loop_access [[LOOP1]]
-// CHECK: store i32 %{{.*}}!llvm.mem.parallel_loop_access [[LOOP1]]
-// CHECK: load i32, i32* %{{.*}}!llvm.mem.parallel_loop_access [[LOOP1]]
+// CHECK: load i32, i32* %{{.*}}!llvm.access.group
+// CHECK: store i32 %{{.*}}!llvm.access.group
+// CHECK: load i32, i32* %{{.*}}!llvm.access.group
 // CHECK: add nsw i32 %{{.+}}, 1
-// CHECK: store i32 %{{.+}}, i32* %{{.*}}!llvm.mem.parallel_loop_access [[LOOP1]]
-// CHECK: br label %{{.*}}!llvm.loop [[LOOP1]]
+// CHECK: store i32 %{{.+}}, i32* %{{.*}}!llvm.access.group
+// CHECK: br label %{{.*}}!llvm.loop
 // CHECK: ret i32 0
 
 // CHECK: define internal i32 [[TASK2]](
@@ -113,17 +113,17 @@
 // CHECK: [[LB_I32:%.+]] = trunc i64 [[LB_VAL]] to i32
 // CHECK: store i32 [[LB_I32]], i32* [[CNT:%.+]],
 // CHECK: br label
-// CHECK: [[VAL:%.+]] = load i32, i32* [[CNT]],{{.*}}!llvm.mem.parallel_loop_access [[LOOP2:!.+]]
+// CHECK: [[VAL:%.+]] = load i32, i32* [[CNT]],{{.*}}!llvm.access.group
 // CHECK: [[VAL_I64:%.+]] = sext i32 [[VAL]] to i64
-// CHECK: [[UB_VAL:%.+]] = load i64, i64* [[UB]],{{.*}}!llvm.mem.parallel_loop_access [[LOOP2]]
+// CHECK: [[UB_VAL:%.+]] = load i64, i64* [[UB]],{{.*}}!llvm.access.group
 // CHECK: [[CMP:%.+]] = icmp ule i64 [[VAL_I64]], [[UB_VAL]]
 // CHECK: br i1 [[CMP]], label %{{.+}}, label %{{.+}}
-// CHECK: load i32, i32* %{{.*}}!llvm.mem.parallel_loop_access [[LOOP2]]
-// CHECK: store i32 %{{.*}}!llvm.mem.parallel_loop_access [[LOOP2]]
-// CHECK: load i32, i32* %{{.*}}!llvm.mem.parallel_loop_access [[LOOP2]]
+// CHECK: load i32, i32* %{{.*}}!llvm.access.group
+// CHECK: store i32 %{{.*}}!llvm.access.group
+// CHECK: load i32, i32* %{{.*}}!llvm.access.group
 // CHECK: add nsw i32 %{{.+}}, 1
-// CHECK: store i32 %{{.+}}, i32* %{{.*}}!llvm.mem.parallel_loop_access [[LOOP2]]
-// CHECK: br label %{{.*}}!llvm.loop [[LOOP2]]
+// CHECK: store i32 %{{.+}}, i32* %{{.*}}!llvm.access.group
+// CHECK: br label %{{.*}}!llvm.loop
 // CHECK: ret i32 0
 
 // CHECK: define internal i32 [[TASK3]](
@@ -142,7 +142,7 @@
 // CHECK: [[LB_VAL:%.+]] = load i64, i64* [[LB]],
 // CHECK: store i64 [[LB_VAL]], i64* [[CNT:%.+]],
 // CHECK: br label
-// CHECK-NOT: !llvm.mem.parallel_loop_access
+// CHECK-NOT: !llvm.access.group
 // CHECK: br label %{{.*}}!llvm.loop
 // CHECK: ret i32 0
 
@@ -192,14 +192,14 @@
 // CHECK: [[CMP:%.+]] = icmp ule i64 [[VAL_I64]], [[UB_VAL]]
 // CHECK: br i1 [[CMP]], label %{{.+}}, label %{{.+}}
 // CHECK: load i32, i32* %
-// CHECK-NOT: !llvm.mem.parallel_loop_access
+// CHECK-NOT: !llvm.access.group
 // CHECK: store i32 %
-// CHECK-NOT: !llvm.mem.parallel_loop_access
+// CHECK-NOT: !llvm.access.group
 // CHECK: load i32, i32* %
-// CHECK-NOT: !llvm.mem.parallel_loop_access
+// CHECK-NOT: !llvm.access.group
 // CHECK: add nsw i32 %{{.+}}, 1
 // CHECK: store i32 %{{.+}}, i32* %
-// CHECK-NOT: !llvm.mem.parallel_loop_access
+// CHECK-NOT: !llvm.access.group
 // CHECK: br label %{{.*}}!llvm.loop
 // CHE

[PATCH] D52117: Generate llvm.loop.parallel_accesses instead of llvm.mem.parallel_loop_access metadata.

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur updated this revision to Diff 177320.
Meinersbur marked an inline comment as done.
Meinersbur added a comment.

- Allow multiple access groups per instructions, i.e. an instruction can be in 
multiple access groups. This allows a simple 'union' operation that occurs when 
inlining into another function. A memory access is considered parallel when at 
least one access group is listed in llvm.loop.parallel_accesses. This is 
prioritized over the 'intersect' case for combining instructions which would be 
dual. We only do best-effort here.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D52117

Files:
  docs/LangRef.rst
  include/llvm/Analysis/LoopInfo.h
  include/llvm/Analysis/LoopInfoImpl.h
  include/llvm/Analysis/VectorUtils.h
  include/llvm/IR/LLVMContext.h
  include/llvm/Transforms/Utils/LoopUtils.h
  lib/Analysis/LoopInfo.cpp
  lib/Analysis/VectorUtils.cpp
  lib/IR/LLVMContext.cpp
  lib/Transforms/InstCombine/InstCombineCalls.cpp
  lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
  lib/Transforms/InstCombine/InstCombinePHI.cpp
  lib/Transforms/Scalar/GVNHoist.cpp
  lib/Transforms/Scalar/LoopVersioningLICM.cpp
  lib/Transforms/Scalar/MemCpyOptimizer.cpp
  lib/Transforms/Scalar/SROA.cpp
  lib/Transforms/Scalar/Scalarizer.cpp
  lib/Transforms/Utils/InlineFunction.cpp
  lib/Transforms/Utils/Local.cpp
  lib/Transforms/Utils/LoopUtils.cpp
  lib/Transforms/Utils/SimplifyCFG.cpp
  test/Analysis/LoopInfo/annotated-parallel-complex.ll
  test/Analysis/LoopInfo/annotated-parallel-simple.ll
  test/ThinLTO/X86/lazyload_metadata.ll
  test/Transforms/Inline/parallel-loop-md-callee.ll
  test/Transforms/Inline/parallel-loop-md-merge.ll
  test/Transforms/Inline/parallel-loop-md.ll
  test/Transforms/InstCombine/intersect-accessgroup.ll
  test/Transforms/InstCombine/loadstore-metadata.ll
  test/Transforms/InstCombine/mem-par-metadata-memcpy.ll
  test/Transforms/LoopVectorize/X86/force-ifcvt.ll
  test/Transforms/LoopVectorize/X86/parallel-loops-after-reg2mem.ll
  test/Transforms/LoopVectorize/X86/parallel-loops.ll
  test/Transforms/LoopVectorize/X86/pr34438.ll
  test/Transforms/LoopVectorize/X86/vect.omp.force.ll
  test/Transforms/LoopVectorize/X86/vect.omp.force.small-tc.ll
  test/Transforms/LoopVectorize/X86/vector_max_bandwidth.ll
  test/Transforms/SROA/mem-par-metadata-sroa.ll
  test/Transforms/Scalarizer/basic.ll
  test/Transforms/SimplifyCFG/combine-parallel-mem-md.ll

Index: test/Transforms/SimplifyCFG/combine-parallel-mem-md.ll
===
--- test/Transforms/SimplifyCFG/combine-parallel-mem-md.ll
+++ test/Transforms/SimplifyCFG/combine-parallel-mem-md.ll
@@ -8,39 +8,39 @@
   br label %for.body
 
 ; CHECK-LABEL: @Test
-; CHECK: load i32, i32* {{.*}}, align 4, !llvm.mem.parallel_loop_access !0
-; CHECK: load i32, i32* {{.*}}, align 4, !llvm.mem.parallel_loop_access !0
-; CHECK: store i32 {{.*}}, align 4, !llvm.mem.parallel_loop_access !0
+; CHECK: load i32, i32* {{.*}}, align 4, !llvm.access.group !0
+; CHECK: load i32, i32* {{.*}}, align 4, !llvm.access.group !0
+; CHECK: store i32 {{.*}}, align 4, !llvm.access.group !0
 ; CHECK-NOT: load
 ; CHECK-NOT: store
 
 for.body: ; preds = %cond.end, %entry
   %indvars.iv = phi i64 [ 0, %entry ], [ %indvars.iv.next, %cond.end ]
   %arrayidx = getelementptr inbounds i32, i32* %p, i64 %indvars.iv
-  %0 = load i32, i32* %arrayidx, align 4, !llvm.mem.parallel_loop_access !0
+  %0 = load i32, i32* %arrayidx, align 4, !llvm.access.group !0
   %cmp1 = icmp eq i32 %0, 0
   br i1 %cmp1, label %cond.true, label %cond.false
 
 cond.false:   ; preds = %for.body
   %arrayidx3 = getelementptr inbounds i32, i32* %res, i64 %indvars.iv
-  %v = load i32, i32* %arrayidx3, align 4, !llvm.mem.parallel_loop_access !0
+  %v = load i32, i32* %arrayidx3, align 4, !llvm.access.group !0
   %arrayidx7 = getelementptr inbounds i32, i32* %d, i64 %indvars.iv
-  %1 = load i32, i32* %arrayidx7, align 4, !llvm.mem.parallel_loop_access !0
+  %1 = load i32, i32* %arrayidx7, align 4, !llvm.access.group !0
   %add = add nsw i32 %1, %v
   br label %cond.end
 
 cond.true:   ; preds = %for.body
   %arrayidx4 = getelementptr inbounds i32, i32* %res, i64 %indvars.iv
-  %w = load i32, i32* %arrayidx4, align 4, !llvm.mem.parallel_loop_access !0
+  %w = load i32, i32* %arrayidx4, align 4, !llvm.access.group !0
   %arrayidx8 = getelementptr inbounds i32, i32* %d, i64 %indvars.iv
-  %2 = load i32, i32* %arrayidx8, align 4, !llvm.mem.parallel_loop_access !0
+  %2 = load i32, i32* %arrayidx8, align 4, !llvm.access.group !0
   %add2 = add nsw i32 %2, %w
   br label %cond.end
 
 cond.end: ; preds = %for.body, %cond.false
   %cond = phi i32 [ %add, %cond.false ], [ %add2, %cond.true ]
   %arrayidx9 = getelementptr inbounds i32, i32* %res

[PATCH] D55424: [analyzer] Supply all checkers with a shouldRegister function

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.

Yeah, this part looks easy :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D55424



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

r348662 - [CUDA] Added missing 'inline' for functions defined in a header.

[Artem Belevich via cfe-commits]

Author: tra
Date: Fri Dec  7 14:20:53 2018
New Revision: 348662

URL: http://llvm.org/viewvc/llvm-project?rev=348662&view=rev
Log:
[CUDA] Added missing 'inline' for functions defined in a header.

Modified:
cfe/trunk/lib/Headers/cuda_wrappers/new

Modified: cfe/trunk/lib/Headers/cuda_wrappers/new
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/cuda_wrappers/new?rev=348662&r1=348661&r2=348662&view=diff
==
--- cfe/trunk/lib/Headers/cuda_wrappers/new (original)
+++ cfe/trunk/lib/Headers/cuda_wrappers/new Fri Dec  7 14:20:53 2018
@@ -73,10 +73,12 @@ __device__ inline void operator delete[]
 
 // Sized delete, C++14 only.
 #if __cplusplus >= 201402L
-__device__ void operator delete(void *ptr, __SIZE_TYPE__ size) CUDA_NOEXCEPT {
+__device__ inline void operator delete(void *ptr,
+   __SIZE_TYPE__ size) CUDA_NOEXCEPT {
   ::operator delete(ptr);
 }
-__device__ void operator delete[](void *ptr, __SIZE_TYPE__ size) CUDA_NOEXCEPT 
{
+__device__ inline void operator delete[](void *ptr,
+ __SIZE_TYPE__ size) CUDA_NOEXCEPT {
   ::operator delete(ptr);
 }
 #endif


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

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: docs/ReleaseNotes.rst:173
+
+  This check adds ``[[nodiscard]]`` attributes (introduced in C++17) to member
+  functions to highlight at compile time where the return value of a function

Please omit this check. Same in documentation.



Comment at: docs/ReleaseNotes.rst:175
+  functions to highlight at compile time where the return value of a function
+  should not be ignored
+

Please add dot at the end.


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

https://reviews.llvm.org/D55433



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

[PATCH] D55425: [analyzer] Split unix.API up to UnixAPIMisuseChecker and UnixAPIPortabilityChecker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: test/Analysis/Inputs/expected-plists/unix-fns.c.plist:3018
  
-  /clang/test/Analysis/unix-fns.c
+  
/home/szelethus/Documents/analyzer_opts/clang/test/Analysis/unix-fns.c
  

Szelethus wrote:
> NoQ wrote:
> > Does this actually work?
> Uhh, didnt catch this one. It does on my own pc and in the office as well, 
> where I use a completely different directory structure.
George's diff filters probably somehow filter this out.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55425



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

[PATCH] D55425: [analyzer] Split unix.API up to UnixAPIMisuseChecker and UnixAPIPortabilityChecker

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

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



Comment at: test/Analysis/Inputs/expected-plists/unix-fns.c.plist:3018
  
-  /clang/test/Analysis/unix-fns.c
+  
/home/szelethus/Documents/analyzer_opts/clang/test/Analysis/unix-fns.c
  

NoQ wrote:
> Does this actually work?
Uhh, didnt catch this one. It does on my own pc and in the office as well, 
where I use a completely different directory structure.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55425



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

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 177313.
MyDeveloperDay added a comment.

Move the conditional statements into AST_MATCHERS


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

https://reviews.llvm.org/D55433

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/UseNodiscardCheck.cpp
  clang-tidy/modernize/UseNodiscardCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-use-nodiscard.rst
  test/clang-tidy/modernize-use-nodiscard.cpp

Index: test/clang-tidy/modernize-use-nodiscard.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-nodiscard.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s modernize-use-nodiscard %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-nodiscard.ReplacementString, value: 'NO_DISCARD'}]}" \
+// RUN: -- -std=c++17 \
+
+// TODO using macros to avoid CHECK-FIXES not being able to find [[xxx]]
+#define MUST_USE_RESULT __attribute__((warn_unused_result))
+#define NO_DISCARD [[nodiscard]]
+#define NO_RETURN [[noreturn]]
+
+class Foo
+{
+public:
+bool f1() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f1' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f1() const;
+
+bool f2(int) const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f2' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f2(int) const;
+
+bool f3(const int &) const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f3' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f3(const int &) const;
+
+bool f4(void) const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f4' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f4(void) const;
+
+// negative tests
+
+void f5() const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: void f5() const;
+
+bool f6();
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool f6();
+
+bool f7(int &);
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool f7(int &);
+
+bool f8(int &) const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool f8(int &) const;
+
+bool f9(int *) const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool f9(int *) const;
+
+bool f10(const int &,int &) const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool f10(const int &,int &) const;
+
+NO_DISCARD bool f12() const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: NO_DISCARD bool f12() const;
+
+MUST_USE_RESULT bool f13() const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: MUST_USE_RESULT bool f13() const;
+   
+// TODO fix CHECK-FIXES to handle "[[" "]]"
+[[nodiscard]] bool f11() const;
+// CHECK-MESSAGES-NOT: warning:
+// _CHECK-FIXES: [[nodiscard]] bool f11() const;
+
+bool _f20() const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool _f20() const;
+
+NO_RETURN bool f21() const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: NO_RETURN bool f21() const;
+
+~Foo();
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: ~Foo();
+
+bool operator +=(int) const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool operator +=(int) const;
+   
+// extra keywords (virtual,inline,const) on return type
+
+virtual bool f14() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f14' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD virtual bool f14() const;
+
+const bool f15() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f15' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD const bool f15() const;
+
+inline const bool f16() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f16' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD inline const bool f16() const;
+
+inline virtual const bool f17() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f17' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD inline virtual const bool f17() const;
+
+// inline with body
+bool f18() const {
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f18' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f18() const {
+return true;
+}
+
+bool f19() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f19' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f19() const;
+};
+
+bool Foo::f19() const
+// CHECK-MESSAGES-NOT: warning:
+// CHEC

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 177312.
MyDeveloperDay added a comment.

Move the conditional checks into matchers


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

https://reviews.llvm.org/D55433

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/UseNodiscardCheck.cpp
  clang-tidy/modernize/UseNodiscardCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-use-nodiscard.rst
  test/clang-tidy/modernize-use-nodiscard.cpp

Index: test/clang-tidy/modernize-use-nodiscard.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-nodiscard.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s modernize-use-nodiscard %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-nodiscard.ReplacementString, value: 'NO_DISCARD'}]}" \
+// RUN: -- -std=c++17 \
+
+// TODO using macros to avoid CHECK-FIXES not being able to find [[xxx]]
+#define MUST_USE_RESULT __attribute__((warn_unused_result))
+#define NO_DISCARD [[nodiscard]]
+#define NO_RETURN [[noreturn]]
+
+class Foo
+{
+public:
+bool f1() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f1' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f1() const;
+
+bool f2(int) const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f2' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f2(int) const;
+
+bool f3(const int &) const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f3' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f3(const int &) const;
+
+bool f4(void) const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f4' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f4(void) const;
+
+// negative tests
+
+void f5() const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: void f5() const;
+
+bool f6();
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool f6();
+
+bool f7(int &);
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool f7(int &);
+
+bool f8(int &) const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool f8(int &) const;
+
+bool f9(int *) const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool f9(int *) const;
+
+bool f10(const int &,int &) const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool f10(const int &,int &) const;
+
+NO_DISCARD bool f12() const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: NO_DISCARD bool f12() const;
+
+MUST_USE_RESULT bool f13() const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: MUST_USE_RESULT bool f13() const;
+   
+// TODO fix CHECK-FIXES to handle "[[" "]]"
+[[nodiscard]] bool f11() const;
+// CHECK-MESSAGES-NOT: warning:
+// _CHECK-FIXES: [[nodiscard]] bool f11() const;
+
+bool _f20() const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool _f20() const;
+
+NO_RETURN bool f21() const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: NO_RETURN bool f21() const;
+
+~Foo();
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: ~Foo();
+
+bool operator +=(int) const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool operator +=(int) const;
+   
+// extra keywords (virtual,inline,const) on return type
+
+virtual bool f14() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f14' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD virtual bool f14() const;
+
+const bool f15() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f15' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD const bool f15() const;
+
+inline const bool f16() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f16' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD inline const bool f16() const;
+
+inline virtual const bool f17() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f17' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD inline virtual const bool f17() const;
+
+// inline with body
+bool f18() const {
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f18' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f18() const {
+return true;
+}
+
+bool f19() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f19' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f19() const;
+};
+
+bool Foo::f19() const
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES:

[PATCH] D55425: [analyzer] Split unix.API up to UnixAPIMisuseChecker and UnixAPIPortabilityChecker

[Artem Dergachev via Phabricator via cfe-commits]

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

Nice!




Comment at: test/Analysis/Inputs/expected-plists/unix-fns.c.plist:3018
  
-  /clang/test/Analysis/unix-fns.c
+  
/home/szelethus/Documents/analyzer_opts/clang/test/Analysis/unix-fns.c
  

Does this actually work?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55425



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

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 177311.
MyDeveloperDay marked 3 inline comments as done.
MyDeveloperDay added a comment.

Move more of the conditional checks into the matchers


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

https://reviews.llvm.org/D55433

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/UseNodiscardCheck.cpp
  clang-tidy/modernize/UseNodiscardCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-use-nodiscard.rst
  test/clang-tidy/modernize-use-nodiscard.cpp

Index: test/clang-tidy/modernize-use-nodiscard.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-nodiscard.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s modernize-use-nodiscard %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-nodiscard.ReplacementString, value: 'NO_DISCARD'}]}" \
+// RUN: -- -std=c++17 \
+
+// TODO using macros to avoid CHECK-FIXES not being able to find [[xxx]]
+#define MUST_USE_RESULT __attribute__((warn_unused_result))
+#define NO_DISCARD [[nodiscard]]
+#define NO_RETURN [[noreturn]]
+
+class Foo
+{
+public:
+bool f1() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f1' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f1() const;
+
+bool f2(int) const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f2' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f2(int) const;
+
+bool f3(const int &) const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f3' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f3(const int &) const;
+
+bool f4(void) const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f4' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f4(void) const;
+
+// negative tests
+
+void f5() const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: void f5() const;
+
+bool f6();
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool f6();
+
+bool f7(int &);
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool f7(int &);
+
+bool f8(int &) const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool f8(int &) const;
+
+bool f9(int *) const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool f9(int *) const;
+
+bool f10(const int &,int &) const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool f10(const int &,int &) const;
+
+NO_DISCARD bool f12() const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: NO_DISCARD bool f12() const;
+
+MUST_USE_RESULT bool f13() const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: MUST_USE_RESULT bool f13() const;
+   
+// TODO fix CHECK-FIXES to handle "[[" "]]"
+[[nodiscard]] bool f11() const;
+// CHECK-MESSAGES-NOT: warning:
+// _CHECK-FIXES: [[nodiscard]] bool f11() const;
+
+bool _f20() const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool _f20() const;
+
+NO_RETURN bool f21() const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: NO_RETURN bool f21() const;
+
+~Foo();
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: ~Foo();
+
+bool operator +=(int) const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool operator +=(int) const;
+   
+// extra keywords (virtual,inline,const) on return type
+
+virtual bool f14() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f14' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD virtual bool f14() const;
+
+const bool f15() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f15' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD const bool f15() const;
+
+inline const bool f16() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f16' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD inline const bool f16() const;
+
+inline virtual const bool f17() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f17' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD inline virtual const bool f17() const;
+
+// inline with body
+bool f18() const {
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f18' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f18() const {
+return true;
+}
+
+bool f19() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f19' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f19() const;
+};
+
+bool Foo::f19() co

Re: [PATCH] D55456: [CUDA] added missing 'inline' for the functions defined in the header.

[Justin Lebar via cfe-commits]

Lgtm

On Fri, Dec 7, 2018, 1:12 PM Artem Belevich via Phabricator <
revi...@reviews.llvm.org> wrote:

> tra created this revision.
> tra added a reviewer: jlebar.
> Herald added subscribers: bixia, sanjoy.
>
> https://reviews.llvm.org/D55456
>
> Files:
>   clang/lib/Headers/cuda_wrappers/new
>
>
> Index: clang/lib/Headers/cuda_wrappers/new
> ===
> --- clang/lib/Headers/cuda_wrappers/new
> +++ clang/lib/Headers/cuda_wrappers/new
> @@ -73,10 +73,12 @@
>
>  // Sized delete, C++14 only.
>  #if __cplusplus >= 201402L
> -__device__ void operator delete(void *ptr, __SIZE_TYPE__ size)
> CUDA_NOEXCEPT {
> +__device__ inline void operator delete(void *ptr,
> +   __SIZE_TYPE__ size) CUDA_NOEXCEPT {
>::operator delete(ptr);
>  }
> -__device__ void operator delete[](void *ptr, __SIZE_TYPE__ size)
> CUDA_NOEXCEPT {
> +__device__ inline void operator delete[](void *ptr,
> + __SIZE_TYPE__ size)
> CUDA_NOEXCEPT {
>::operator delete(ptr);
>  }
>  #endif
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55349: Convert some ObjC alloc msgSends to runtime calls

[John McCall via Phabricator via cfe-commits]

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

LGTM.


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

https://reviews.llvm.org/D55349



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

[PATCH] D55388: [analyzer] MoveChecker Pt.8: Add checks for dereferencing a smart pointer after move.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 177307.
NoQ marked an inline comment as done.
NoQ added a comment.

Fxd!


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

https://reviews.llvm.org/D55388

Files:
  lib/StaticAnalyzer/Checkers/MoveChecker.cpp
  test/Analysis/use-after-move.cpp

Index: test/Analysis/use-after-move.cpp
===
--- test/Analysis/use-after-move.cpp
+++ test/Analysis/use-after-move.cpp
@@ -1,20 +1,26 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.Move -verify %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
-// RUN:  -analyzer-config exploration_strategy=unexplored_first_queue
+// RUN:  -analyzer-config exploration_strategy=unexplored_first_queue\
+// RUN:  -analyzer-checker debug.ExprInspection
 // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.Move -verify %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
-// RUN:  -analyzer-config exploration_strategy=dfs -DDFS=1
+// RUN:  -analyzer-config exploration_strategy=dfs -DDFS=1\
+// RUN:  -analyzer-checker debug.ExprInspection
 // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.Move -verify %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
 // RUN:  -analyzer-config exploration_strategy=unexplored_first_queue\
-// RUN:  -analyzer-config alpha.cplusplus.Move:Aggressive=true -DAGGRESSIVE
+// RUN:  -analyzer-config alpha.cplusplus.Move:Aggressive=true -DAGGRESSIVE\
+// RUN:  -analyzer-checker debug.ExprInspection
 // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.Move -verify %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
 // RUN:  -analyzer-config exploration_strategy=dfs -DDFS=1\
-// RUN:  -analyzer-config alpha.cplusplus.Move:Aggressive=true -DAGGRESSIVE
+// RUN:  -analyzer-config alpha.cplusplus.Move:Aggressive=true -DAGGRESSIVE\
+// RUN:  -analyzer-checker debug.ExprInspection
 
 #include "Inputs/system-header-simulator-cxx.h"
 
+void clang_analyzer_warnIfReached();
+
 class B {
 public:
   B() = default;
@@ -810,7 +816,19 @@
 // expected-note@-4{{Object 'P' is moved}}
 // expected-note@-4{{Method called on moved-from object 'P'}}
 #endif
-*P += 1; // FIXME: Should warn that the pointer is null.
+
+// Because that well-defined state is null, dereference is still UB.
+// Note that in aggressive mode we already warned about 'P',
+// so no extra warning is generated.
+*P += 1;
+#ifndef AGGRESSIVE
+// expected-warning@-2{{Dereference of null smart pointer 'P' of type 'std::unique_ptr'}}
+// expected-note@-14{{Smart pointer 'P' of type 'std::unique_ptr' is reset to null when moved from}}
+// expected-note@-4{{Dereference of null smart pointer 'P' of type 'std::unique_ptr'}}
+#endif
+
+// The program should have crashed by now.
+clang_analyzer_warnIfReached(); // no-warning
   }
 };
 
@@ -827,3 +845,9 @@
   P.get(); // expected-warning{{Method called on moved-from object 'P'}}
// expected-note@-1{{Method called on moved-from object 'P'}}
 }
+
+void localUniquePtrWithArrow(std::unique_ptr P) {
+  std::unique_ptr Q = std::move(P); // expected-note{{Smart pointer 'P' of type 'std::unique_ptr' is reset to null when moved from}}
+  P->foo(); // expected-warning{{Dereference of null smart pointer 'P' of type 'std::unique_ptr'}}
+// expected-note@-1{{Dereference of null smart pointer 'P' of type 'std::unique_ptr'}}
+}
Index: lib/StaticAnalyzer/Checkers/MoveChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MoveChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MoveChecker.cpp
@@ -61,19 +61,35 @@
   const char *NL, const char *Sep) const override;
 
 private:
-  enum MisuseKind { MK_FunCall, MK_Copy, MK_Move };
+  enum MisuseKind { MK_FunCall, MK_Copy, MK_Move, MK_Dereference };
+  enum StdObjectKind { SK_NonStd, SK_Unsafe, SK_Safe, SK_SmartPtr };
+
+  static bool misuseCausesCrash(MisuseKind MK) {
+return MK == MK_Dereference;
+  }
 
   struct ObjectKind {
-bool Local : 1; // Is this a local variable or a local rvalue reference?
-bool STL : 1; // Is this an object of a standard type?
+// Is this a local variable or a local rvalue reference?
+bool IsLocal : 1;
+// Is this an STL object? If so, of what kind?
+StdObjectKind StdKind : 2;
+  };
+
+  // STL smart pointers are automatically re-initialized to null when moved
+  // from. So we can't warn on many methods, but we can warn when it is
+  // dereferenced, which is UB even if the resulting lvalue never gets read.
+  const llvm::StringSet<> StdSmartPtrClasses = {
+  "shared_ptr",
+  "unique_ptr",
+  "weak_ptr",
   };
 
   // Not all of these are entirely move-safe, but they do provide *some*
   // guarantees, and it means that somebody is using them after mo

[PATCH] D55349: Convert some ObjC alloc msgSends to runtime calls

[Pete Cooper via Phabricator via cfe-commits]

pete updated this revision to Diff 177308.
pete marked an inline comment as done.
pete added a comment.

Added test for integer argument and updated code to only accept pointer types 
to allocWithZone.


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

https://reviews.llvm.org/D55349

Files:
  include/clang/Basic/ObjCRuntime.h
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/CGObjC.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/CodeGenModule.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGenObjC/convert-messages-to-runtime-calls.m
  test/Driver/objc-convert-messages-to-runtime-calls.m

Index: test/Driver/objc-convert-messages-to-runtime-calls.m
===
--- /dev/null
+++ test/Driver/objc-convert-messages-to-runtime-calls.m
@@ -0,0 +1,7 @@
+// RUN: %clang %s -### -o %t.o 2>&1 -fsyntax-only -fobjc-convert-messages-to-runtime-calls -fno-objc-convert-messages-to-runtime-calls -target x86_64-apple-macosx10.10.0 | FileCheck  %s --check-prefix=DISABLE
+// RUN: %clang %s -### -o %t.o 2>&1 -fsyntax-only -fno-objc-convert-messages-to-runtime-calls -fobjc-convert-messages-to-runtime-calls -target x86_64-apple-macosx10.10.0 | FileCheck  %s --check-prefix=ENABLE
+
+// Check that we pass fobjc-convert-messages-to-runtime-calls only when supported, and not explicitly disabled.
+
+// DISABLE: "-fno-objc-convert-messages-to-runtime-calls"
+// ENABLE-NOT: "-fno-objc-convert-messages-to-runtime-calls"
Index: test/CodeGenObjC/convert-messages-to-runtime-calls.m
===
--- /dev/null
+++ test/CodeGenObjC/convert-messages-to-runtime-calls.m
@@ -0,0 +1,80 @@
+// RUN: %clang_cc1 -fobjc-runtime=macosx-10.10.0 -emit-llvm -o - %s -fno-objc-convert-messages-to-runtime-calls | FileCheck %s --check-prefix=MSGS
+// RUN: %clang_cc1 -fobjc-runtime=macosx-10.10.0 -emit-llvm -o - %s | FileCheck %s --check-prefix=CALLS
+// RUN: %clang_cc1 -fobjc-runtime=macosx-10.9.0 -emit-llvm -o - %s | FileCheck %s --check-prefix=MSGS
+// RUN: %clang_cc1 -fobjc-runtime=macosx-fragile-10.10.0 -emit-llvm -o - %s | FileCheck %s --check-prefix=MSGS
+// RUN: %clang_cc1 -fobjc-runtime=ios-8.0 -emit-llvm -o - %s | FileCheck %s --check-prefix=CALLS
+// RUN: %clang_cc1 -fobjc-runtime=ios-7.0 -emit-llvm -o - %s | FileCheck %s --check-prefix=MSGS
+// Note: This line below is for tvos for which the driver passes through to use the ios9.0 runtime.
+// RUN: %clang_cc1 -fobjc-runtime=ios-9.0 -emit-llvm -o - %s | FileCheck %s --check-prefix=CALLS
+// RUN: %clang_cc1 -fobjc-runtime=watchos-2.0 -emit-llvm -o - %s | FileCheck %s --check-prefix=CALLS
+
+#define nil (id)0
+
+@interface NSObject
++ (id)alloc;
++ (id)allocWithZone:(void*)zone;
++ (id)alloc2;
+@end
+
+// CHECK-LABEL: define {{.*}}void @test1
+void test1(id x) {
+  // MSGS: {{call.*@objc_msgSend}}
+  // MSGS: {{call.*@objc_msgSend}}
+  // CALLS: {{call.*@objc_alloc}}
+  // CALLS: {{call.*@objc_allocWithZone}}
+  [NSObject alloc];
+  [NSObject allocWithZone:nil];
+}
+
+// CHECK-LABEL: define {{.*}}void @test2
+void test2(void* x) {
+  // MSGS: {{call.*@objc_msgSend}}
+  // MSGS: {{call.*@objc_msgSend}}
+  // MSGS: {{call.*@objc_msgSend}}
+  // CALLS: {{call.*@objc_msgSend}}
+  // CALLS: {{call.*@objc_msgSend}}
+  // CALLS: {{call.*@objc_msgSend}}
+  [NSObject alloc2];
+  [NSObject allocWithZone:(void*)-1];
+  [NSObject allocWithZone:x];
+}
+
+@class A;
+@interface B
++ (A*) alloc;
++ (A*)allocWithZone:(void*)zone;
+@end
+
+// Make sure we get a bitcast on the return type as the
+// call will return i8* which we have to cast to A*
+// CHECK-LABEL: define {{.*}}void @test_alloc_class_ptr
+A* test_alloc_class_ptr() {
+  // CALLS: {{call.*@objc_alloc}}
+  // CALLS-NEXT: bitcast i8*
+  // CALLS-NEXT: ret
+  return [B alloc];
+}
+
+// Make sure we get a bitcast on the return type as the
+// call will return i8* which we have to cast to A*
+// CHECK-LABEL: define {{.*}}void @test_alloc_class_ptr
+A* test_allocWithZone_class_ptr() {
+  // CALLS: {{call.*@objc_allocWithZone}}
+  // CALLS-NEXT: bitcast i8*
+  // CALLS-NEXT: ret
+  return [B allocWithZone:nil];
+}
+
+
+@interface C
++ (id)allocWithZone:(int)intArg;
+@end
+
+// Make sure we only accept pointer types
+// CHECK-LABEL: define {{.*}}void @test_allocWithZone_int
+C* test_allocWithZone_int() {
+  // MSGS: {{call.*@objc_msgSend}}
+  // CALLS: {{call.*@objc_msgSend}}
+  return [C allocWithZone:3];
+}
+
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1155,6 +1155,10 @@
 }
   }
 
+
+  if (Args.hasArg(OPT_fno_objc_convert_messages_to_runtime_calls))
+Opts.ObjCConvertMessagesToRuntimeCalls = 0;
+
   if (Args.getLastArg(OPT_femulated_tls) ||
   Args.getLastArg(OPT_fno_emulated_tls)) {
 Opts.Explic

[PATCH] D55387: [analyzer] MoveChecker Pt.7: NFC: Misc refactoring.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 177306.
NoQ marked 2 inline comments as done.
NoQ added a comment.

Fxd.


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

https://reviews.llvm.org/D55387

Files:
  lib/StaticAnalyzer/Checkers/MoveChecker.cpp

Index: lib/StaticAnalyzer/Checkers/MoveChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MoveChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MoveChecker.cpp
@@ -89,6 +89,20 @@
   "weak_ptr",
   };
 
+  // Should we bother tracking the state of the object?
+  bool shouldBeTracked(ObjectKind OK) const {
+// In non-aggressive mode, only warn on use-after-move of local variables
+// (or local rvalue references) and of STL objects. The former is possible
+// because local variables (or local rvalue references) are not tempting
+// their user to re-use the storage. The latter is possible because STL
+// objects are known to end up in a valid but unspecified state after the
+// move and their state-reset methods are also known, which allows us to
+// predict precisely when use-after-move is invalid. In aggressive mode,
+// warn on any use-after-move because the user has intentionally asked us
+// to completely eliminate use-after-move in his code.
+return IsAggressive || OK.Local || OK.STL;
+  }
+
   // Obtains ObjectKind of an object. Because class declaration cannot always
   // be easily obtained from the memory region, it is supplied separately.
   ObjectKind classifyObject(const MemRegion *MR, const CXXRecordDecl *RD) const;
@@ -136,8 +150,20 @@
 
 private:
   mutable std::unique_ptr BT;
+
+  // Check if the given form of potential misuse of a given object
+  // should be reported. If so, get it reported. The callback from which
+  // this function was called should immediately return after the call
+  // because this function adds one or two transitions.
+  void checkUse(ProgramStateRef State, const MemRegion *Region,
+const CXXRecordDecl *RD, MisuseKind MK,
+CheckerContext &C) const;
+
+  // Returns the exploded node against which the report was emitted.
+  // The caller *must* add any further transitions against this node.
   ExplodedNode *reportBug(const MemRegion *Region, const CXXRecordDecl *RD,
   CheckerContext &C, MisuseKind MK) const;
+
   bool isInMoveSafeContext(const LocationContext *LC) const;
   bool isStateResetMethod(const CXXMethodDecl *MethodDec) const;
   bool isMoveSafeMethod(const CXXMethodDecl *MethodDec) const;
@@ -236,6 +262,25 @@
   return MoveNode;
 }
 
+void MoveChecker::checkUse(ProgramStateRef State, const MemRegion *Region,
+   const CXXRecordDecl *RD, MisuseKind MK,
+   CheckerContext &C) const {
+  assert(!C.isDifferent() && "No transitions should have been made by now");
+  const RegionState *RS = State->get(Region);
+
+  if (!RS || isAnyBaseRegionReported(State, Region)
+  || isInMoveSafeContext(C.getLocationContext())) {
+// Finalize changes made by the caller.
+C.addTransition(State);
+return;
+  }
+
+  ExplodedNode *N = reportBug(Region, RD, C, MK);
+
+  State = State->set(Region, RegionState::getReported());
+  C.addTransition(State, N);
+}
+
 ExplodedNode *MoveChecker::reportBug(const MemRegion *Region,
  const CXXRecordDecl *RD,
  CheckerContext &C,
@@ -294,6 +339,8 @@
   if (!MethodDecl)
 return;
 
+  const CXXRecordDecl *RD = MethodDecl->getParent();
+
   const auto *ConstructorDecl = dyn_cast(MethodDecl);
 
   const auto *CC = dyn_cast_or_null(&Call);
@@ -310,20 +357,6 @@
   if (!ArgRegion)
 return;
 
-  // In non-aggressive mode, only warn on use-after-move of local variables (or
-  // local rvalue references) and of STL objects. The former is possible because
-  // local variables (or local rvalue references) are not tempting their user to
-  // re-use the storage. The latter is possible because STL objects are known
-  // to end up in a valid but unspecified state after the move and their
-  // state-reset methods are also known, which allows us to predict
-  // precisely when use-after-move is invalid.
-  // In aggressive mode, warn on any use-after-move because the user
-  // has intentionally asked us to completely eliminate use-after-move
-  // in his code.
-  ObjectKind OK = classifyObject(ArgRegion, MethodDecl->getParent());
-  if (!IsAggressive && !OK.Local && !OK.STL)
-return;
-
   // Skip moving the object to itself.
   if (CC && CC->getCXXThisVal().getAsRegion() == ArgRegion)
 return;
@@ -340,9 +373,15 @@
 
   if (State->get(ArgRegion))
 return;
-  // Mark object as moved-from.
-  State = State->set(ArgRegion, RegionState::getMoved());
-  C.addTransition(State);
+
+  ObjectKind OK = classifyObject(ArgRegion, RD);
+  if (shouldBeTracked(OK)) {
+// Mark object as moved-from.
+S

[PATCH] D55458: [analyzer] ObjCContainers: Track index values.

[Artem Dergachev via Phabricator via cfe-commits]

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



Comment at: test/Analysis/CFContainers.mm:178
 void TestGetCount(CFArrayRef A, CFIndex sIndex) {
-  CFIndex sCount = CFArrayGetCount(A);
-  if (sCount > sIndex)
+  CFIndex sCount = CFArrayGetCount(A); // expected-note{{'sCount' initialized 
here}}
+  if (sCount > sIndex) // expected-note{{Assuming 'sCount' is <= 'sIndex'}}

This is a new note.



Comment at: test/Analysis/CFContainers.mm:240
 void TestCFMutableArrayRefEscapeViaImmutableArgument(CFMutableArrayRef a) {
-  CFIndex aLen = CFArrayGetCount(a);
+  CFIndex aLen = CFArrayGetCount(a); // expected-note{{'aLen' initialized 
here}}
   ArrayRefEscape(a);

This is a new note.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55458



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

[PATCH] D55458: [analyzer] ObjCContainers: Track index values.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, george.karpenkov.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, arphaman, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.

This is a checker for CF (well, not really ObjC) arrays that checks array 
bounds. As such, it's a good place to experiment with array bound checking.

Use `trackExpressionValue()` (previously known as `trackNullOrUndefValue()`) to 
track index value, so that the user knew what Static Analyzer thinks the index 
is.

Hopefully this will make warnings more understandable. Other ideas for better 
warnings include adding index and length values to the message and explaining 
how constraints over index and length evolve along the path.

Additionally, add `printState()` to help debugging the checker later.


Repository:
  rC Clang

https://reviews.llvm.org/D55458

Files:
  lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp
  test/Analysis/CFContainers.mm

Index: test/Analysis/CFContainers.mm
===
--- test/Analysis/CFContainers.mm
+++ test/Analysis/CFContainers.mm
@@ -1,4 +1,7 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=osx.coreFoundation.containers.PointerSizedValues,osx.coreFoundation.containers.OutOfBounds -analyzer-store=region -triple x86_64-apple-darwin -verify %s
+// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin -analyzer-output=text\
+// RUN: -analyzer-checker=osx.coreFoundation.containers.PointerSizedValues\
+// RUN: -analyzer-checker=osx.coreFoundation.containers.OutOfBounds\
+// RUN: -verify %s
 
 typedef const struct __CFAllocator * CFAllocatorRef;
 typedef const struct __CFString * CFStringRef;
@@ -94,17 +97,21 @@
 #define CFSTR(cStr)  ((CFStringRef) __builtin___CFStringMakeConstantString ("" cStr ""))
 #define NULL __null
 
-// Done with the headers. 
+// Done with the headers.
 // Test alpha.osx.cocoa.ContainerAPI checker.
 void testContainers(int **xNoWarn, CFIndex count) {
   int x[] = { 1, 2, 3 };
-  CFArrayRef foo = CFArrayCreate(kCFAllocatorDefault, (const void **) x, sizeof(x) / sizeof(x[0]), 0);// expected-warning {{The second argument to 'CFArrayCreate' must be a C array of pointer-sized}}
+  CFArrayRef foo = CFArrayCreate(kCFAllocatorDefault, (const void **) x, sizeof(x) / sizeof(x[0]), 0);
+  // expected-warning@-1 {{The second argument to 'CFArrayCreate' must be a C array of pointer-sized}}
+  // expected-note@-2{{The second argument to 'CFArrayCreate' must be a C array of pointer-sized}}
 
   CFArrayRef fooNoWarn = CFArrayCreate(kCFAllocatorDefault, (const void **) xNoWarn, sizeof(xNoWarn) / sizeof(xNoWarn[0]), 0); // no warning
   CFArrayRef fooNoWarn2 = CFArrayCreate(kCFAllocatorDefault, 0, sizeof(xNoWarn) / sizeof(xNoWarn[0]), 0);// no warning, passing in 0
   CFArrayRef fooNoWarn3 = CFArrayCreate(kCFAllocatorDefault, NULL, sizeof(xNoWarn) / sizeof(xNoWarn[0]), 0);// no warning, passing in NULL
 
-  CFSetRef set = CFSetCreate(NULL, (const void **)x, 3, &kCFTypeSetCallBacks); // expected-warning {{The second argument to 'CFSetCreate' must be a C array of pointer-sized values}}
+  CFSetRef set = CFSetCreate(NULL, (const void **)x, 3, &kCFTypeSetCallBacks);
+  // expected-warning@-1 {{The second argument to 'CFSetCreate' must be a C array of pointer-sized values}}
+  // expected-note@-2{{The second argument to 'CFSetCreate' must be a C array of pointer-sized values}}
   CFArrayRef* pairs = new CFArrayRef[count];
   CFSetRef fSet = CFSetCreate(kCFAllocatorDefault, (const void**) pairs, count - 1, &kCFTypeSetCallBacks);// no warning
 }
@@ -126,8 +133,13 @@
   const CFDictionaryKeyCallBacks keyCB = kCFCopyStringDictionaryKeyCallBacks;
   const CFDictionaryValueCallBacks valCB = kCFTypeDictionaryValueCallBacks;
   CFDictionaryRef dict1 = CFDictionaryCreate(kCFAllocatorDefault, (const void**)keys, (const void**)values, numValues, &keyCB, &valCB); // no warning
-  CFDictionaryRef dict2 = CFDictionaryCreate(kCFAllocatorDefault, (const void**)elems[0], (const void**)values, numValues, &keyCB, &valCB); //expected-warning {{The second argument to 'CFDictionaryCreate' must be a C array of}} expected-warning {{cast to 'const void **' from smaller integer type 'int'}}
-  CFDictionaryRef dict3 = CFDictionaryCreate(kCFAllocatorDefault, (const void**)keys, (const void**)elems, numValues, &keyCB, &valCB); // expected-warning {{The third argument to 'CFDictionaryCreate' must be a C array of pointer-sized values}}
+  CFDictionaryRef dict2 = CFDictionaryCreate(kCFAllocatorDefault, (const void**)elems[0], (const void**)values, numValues, &keyCB, &valCB);
+  // expected-warning@-1 {{The second argument to 'CFDictionaryCreate' must be a C array of}}
+  // expected-note@-2{{The second argument to 'CFDictionaryCreate' must be a C array of}}
+  // expected-warning@-3{{cast to 'const void **' from smaller integer type 'int'}}
+  CFDictionaryRef dict3 = CFDictionaryCreate(kCFAllocatorDefault, (const void*

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay marked 12 inline comments as done.
MyDeveloperDay added inline comments.



Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:28-30
+bool isOperator(const FunctionDecl *D) {
+  return D->getNameAsString().find("operator") != std::string::npos;
+}

lebedev.ri wrote:
> Can't you use `isOverloadedOperator()`?
> No way going to `std::string` is fast.
Great! I didn't even know that existed (this is my first checker so I'm still 
learning what I can do), I tried to use it in the matcher but I couldn't get it 
to work, (see comment below about your matcher comment)



Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:32-34
+bool isInternalFunction(const FunctionDecl *D) {
+  return D->getNameAsString().find("_") == 0;
+}

lebedev.ri wrote:
> Motivation?
> This should be configurable in some way.
this was because when I ran it on my source tree with -fix it, it wanted to try 
and fix the windows headers, and that scared me a bit! so I was trying to 
prevent it stepping into standard headers etc.. I'll make a change to make that 
optional



Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:47
+void UseNodiscardCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus17)
+return;

lebedev.ri wrote:
> This should be smarter, since you provide `ReplacementString`.
> E.g. if `ReplacementString` is default, require C++17.
> Else, only require C++.
That's a great idea, my team can barely get onto all of C++ 11 because some 
platform compilers are SO far behind, but I really like that idea of allowing 
it to still work if using a macro



Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:63-95
+  // if the location is invalid forget it
+  if (!MatchedDecl->getLocation().isValid())
+return;
+
+  if (MatchedDecl->isThisDeclarationADefinition() && 
MatchedDecl->isOutOfLine())
+return;
+

lebedev.ri wrote:
> All this should be done in `registerMatchers()`.
Ok, I think this is my naivety on how to actually do it, because I tried but 
often the functions on MatchDecl->isXXX() didn't always map 1-to-1 with what 
the matchers were expecting (but most likely I was just doing it wrong), let me 
swing back round once I've read some more from @stephenkelly blogs



Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:108
+  << MatchedDecl
+  << FixItHint::CreateInsertion(retLoc, NoDiscardMacro + " ");
+  return;

lebedev.ri wrote:
> Can you actually provide fix-it though?
> If the result will be unused, it will break build under `-Werror`.
It is true, it will generate warnings if people aren't using it, does that go 
against what people traditionally believe clang-tidy should do? I mean I get it 
that clang-tidy should mostly tidy the code and leave it in a working state, 
but is causing people to break their eggs considered a no-no? again this might 
be my naivety about what clang-tidy remit is.



Comment at: docs/clang-tidy/checks/modernize-use-nodiscard.rst:6
+
+This check adds ``[[nodiscard]]`` attributes (introduced in C++17) to member 
functions to highlight at compile time where the return value of a function 
should not be ignored
+

lebedev.ri wrote:
> Eugene.Zelenko wrote:
> > Please use 80 characters limit.
> Same, the rules need to be spelled out, this sounds a bit too magical right 
> now,
I'm not a great wordsmith..but is this too wordy for what you like in the 
Docs/ReleaseNotes?


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

https://reviews.llvm.org/D55433



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

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:40
+void UseNodiscardCheck::registerMatchers(MatchFinder *Finder) {
+
+  // if we are using C++17 attributes we are going to need c++17

Unnecessary empty line.



Comment at: docs/ReleaseNotes.rst:173
+
+  Checks to detect if a member function looks like its return type should not 
+  be ignored.

One sentence is enough for Release Notes. Will be good idea to make it same as 
in documentation.



Comment at: docs/clang-tidy/checks/modernize-use-nodiscard.rst:40
+
+Users can use :option:`ReplacementString` to specify a macro to use instead of
+``[[nodiscard]]``. This is useful when maintaining source code that needs to

May be just this option?


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

https://reviews.llvm.org/D55433



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

[PATCH] D55349: Convert some ObjC alloc msgSends to runtime calls

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/CodeGen/CGObjC.cpp:385
+// prefix are identified as OMF_alloc but we only want to call the
+// runtime for this version.
+if (Sel.isUnarySelector() && Sel.getNameForSlot(0) == "alloc")

Actually, I'd suggest just rewriting this comment to look like the next one, it 
conveys the same idea but much more succinctly.



Comment at: lib/CodeGen/CGObjC.cpp:390
+if (Sel.isKeywordSelector() && Sel.getNumArgs() == 1 &&
+Args.size() == 1 && Args.front().getKnownRValue().isScalar() &&
+Sel.getNameForSlot(0) == "allocWithZone") {

You should check `!Args.front().hasLValue()` here before calling 
`getKnownRValue()`.  The test case would be some silly example where 
`-allocWithZone:` is declared with a parameter that's some big `struct` and the 
argument expression just loads it immediately from an l-value.  Or you could 
just check that the parameter type is a pointer type.


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

https://reviews.llvm.org/D55349



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

[PATCH] D55455: Remove stat cache chaining as it's no longer needed after PTH support has been removed

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

This patch LGTM, but before committing I suggest sending an RFC to cfe-dev and 
waiting a few days.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55455



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

[PATCH] D55455: Remove stat cache chaining as it's no longer needed after PTH support has been removed

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

I've no problem with this, but I don't consider myself 'expert' on this stuff 
enough to mark 'Approve' without giving others time to take a look.  I'd say 
give this until this time tuesday for the others to take a look, then you can 
consider this an 'approve'.

I won't mark it as such so as to not scare away other reviews :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D55455



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

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 177295.
MyDeveloperDay added a comment.

Address some (not all yet) review comments


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

https://reviews.llvm.org/D55433

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/UseNodiscardCheck.cpp
  clang-tidy/modernize/UseNodiscardCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-use-nodiscard.rst
  test/clang-tidy/modernize-use-nodiscard.cpp

Index: test/clang-tidy/modernize-use-nodiscard.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-nodiscard.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s modernize-use-nodiscard %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-nodiscard.ReplacementString, value: 'NO_DISCARD'}]}" \
+// RUN: -- -std=c++17 \
+
+// TODO using macros to avoid CHECK-FIXES not being able to find [[xxx]]
+#define MUST_USE_RESULT __attribute__((warn_unused_result))
+#define NO_DISCARD [[nodiscard]]
+#define NO_RETURN [[noreturn]]
+
+class Foo
+{
+public:
+bool f1() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f1' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f1() const;
+
+bool f2(int) const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f2' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f2(int) const;
+
+bool f3(const int &) const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f3' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f3(const int &) const;
+
+bool f4(void) const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f4' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f4(void) const;
+
+// negative tests
+
+void f5() const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: void f5() const;
+
+bool f6();
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool f6();
+
+bool f7(int &);
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool f7(int &);
+
+bool f8(int &) const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool f8(int &) const;
+
+bool f9(int *) const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool f9(int *) const;
+
+bool f10(const int &,int &) const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool f10(const int &,int &) const;
+
+NO_DISCARD bool f12() const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: NO_DISCARD bool f12() const;
+
+MUST_USE_RESULT bool f13() const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: MUST_USE_RESULT bool f13() const;
+   
+// TODO fix CHECK-FIXES to handle "[[" "]]"
+[[nodiscard]] bool f11() const;
+// CHECK-MESSAGES-NOT: warning:
+// _CHECK-FIXES: [[nodiscard]] bool f11() const;
+
+bool _f20() const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool _f20() const;
+
+NO_RETURN bool f21() const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: NO_RETURN bool f21() const;
+
+~Foo();
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: ~Foo();
+
+bool operator +=(int) const;
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool operator +=(int) const;
+   
+// extra keywords (virtual,inline,const) on return type
+
+virtual bool f14() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f14' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD virtual bool f14() const;
+
+const bool f15() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f15' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD const bool f15() const;
+
+inline const bool f16() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f16' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD inline const bool f16() const;
+
+inline virtual const bool f17() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f17' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD inline virtual const bool f17() const;
+
+// inline with body
+bool f18() const {
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f18' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f18() const {
+return true;
+}
+
+bool f19() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f19' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f19() const;
+};
+
+bool Foo::f19() const
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES

[PATCH] D55456: [CUDA] added missing 'inline' for the functions defined in the header.

[Artem Belevich via Phabricator via cfe-commits]

tra created this revision.
tra added a reviewer: jlebar.
Herald added subscribers: bixia, sanjoy.

https://reviews.llvm.org/D55456

Files:
  clang/lib/Headers/cuda_wrappers/new


Index: clang/lib/Headers/cuda_wrappers/new
===
--- clang/lib/Headers/cuda_wrappers/new
+++ clang/lib/Headers/cuda_wrappers/new
@@ -73,10 +73,12 @@
 
 // Sized delete, C++14 only.
 #if __cplusplus >= 201402L
-__device__ void operator delete(void *ptr, __SIZE_TYPE__ size) CUDA_NOEXCEPT {
+__device__ inline void operator delete(void *ptr,
+   __SIZE_TYPE__ size) CUDA_NOEXCEPT {
   ::operator delete(ptr);
 }
-__device__ void operator delete[](void *ptr, __SIZE_TYPE__ size) CUDA_NOEXCEPT 
{
+__device__ inline void operator delete[](void *ptr,
+ __SIZE_TYPE__ size) CUDA_NOEXCEPT {
   ::operator delete(ptr);
 }
 #endif


Index: clang/lib/Headers/cuda_wrappers/new
===
--- clang/lib/Headers/cuda_wrappers/new
+++ clang/lib/Headers/cuda_wrappers/new
@@ -73,10 +73,12 @@
 
 // Sized delete, C++14 only.
 #if __cplusplus >= 201402L
-__device__ void operator delete(void *ptr, __SIZE_TYPE__ size) CUDA_NOEXCEPT {
+__device__ inline void operator delete(void *ptr,
+   __SIZE_TYPE__ size) CUDA_NOEXCEPT {
   ::operator delete(ptr);
 }
-__device__ void operator delete[](void *ptr, __SIZE_TYPE__ size) CUDA_NOEXCEPT {
+__device__ inline void operator delete[](void *ptr,
+ __SIZE_TYPE__ size) CUDA_NOEXCEPT {
   ::operator delete(ptr);
 }
 #endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55455: Remove stat cache chaining as it's no longer needed after PTH support has been removed

[Alex Lorenz via Phabricator via cfe-commits]

arphaman created this revision.
arphaman added reviewers: erichkeane, dexonsmith, vsapsai, Bigcheese, 
ilya-biryukov.
Herald added a subscriber: jkorous.

Stat cache chaining was implemented for a StatListener in the PTH writer so 
that it could write out the stat information to PTH. r348266 removed support 
for PTH, and it doesn't seem like there are other uses of stat cache chaining. 
We can remove the chaining support.


Repository:
  rC Clang

https://reviews.llvm.org/D55455

Files:
  include/clang/Basic/FileManager.h
  include/clang/Basic/FileSystemStatCache.h
  lib/Basic/FileManager.cpp
  lib/Basic/FileSystemStatCache.cpp
  lib/Tooling/Tooling.cpp
  unittests/Basic/FileManagerTest.cpp

Index: unittests/Basic/FileManagerTest.cpp
===
--- unittests/Basic/FileManagerTest.cpp
+++ unittests/Basic/FileManagerTest.cpp
@@ -111,7 +111,7 @@
   // FileManager to report "file/directory doesn't exist".  This
   // avoids the possibility of the result of this test being affected
   // by what's in the real file system.
-  manager.addStatCache(llvm::make_unique());
+  manager.setStatCache(llvm::make_unique());
 
   EXPECT_EQ(nullptr, manager.getDirectory("virtual/dir/foo"));
   EXPECT_EQ(nullptr, manager.getDirectory("virtual/dir"));
@@ -121,7 +121,7 @@
 // When a virtual file is added, all of its ancestors should be created.
 TEST_F(FileManagerTest, getVirtualFileCreatesDirectoryEntriesForAncestors) {
   // Fake an empty real file system.
-  manager.addStatCache(llvm::make_unique());
+  manager.setStatCache(llvm::make_unique());
 
   manager.getVirtualFile("virtual/dir/bar.h", 100, 0);
   EXPECT_EQ(nullptr, manager.getDirectory("virtual/dir/foo"));
@@ -149,7 +149,7 @@
   statCache->InjectFile(FileName, 45);
 #endif
 
-  manager.addStatCache(std::move(statCache));
+  manager.setStatCache(std::move(statCache));
 
   const FileEntry *file = manager.getFile("/tmp/test");
   ASSERT_TRUE(file != nullptr);
@@ -173,7 +173,7 @@
 // getFile() returns non-NULL if a virtual file exists at the given path.
 TEST_F(FileManagerTest, getFileReturnsValidFileEntryForExistingVirtualFile) {
   // Fake an empty real file system.
-  manager.addStatCache(llvm::make_unique());
+  manager.setStatCache(llvm::make_unique());
 
   manager.getVirtualFile("virtual/dir/bar.h", 100, 0);
   const FileEntry *file = manager.getFile("virtual/dir/bar.h");
@@ -195,7 +195,7 @@
   statCache->InjectDirectory(".", 41);
   statCache->InjectFile("foo.cpp", 42);
   statCache->InjectFile("bar.cpp", 43);
-  manager.addStatCache(std::move(statCache));
+  manager.setStatCache(std::move(statCache));
 
   const FileEntry *fileFoo = manager.getFile("foo.cpp");
   const FileEntry *fileBar = manager.getFile("bar.cpp");
@@ -213,7 +213,7 @@
   auto statCache = llvm::make_unique();
   statCache->InjectDirectory(".", 41);
   statCache->InjectFile("foo.cpp", 42);
-  manager.addStatCache(std::move(statCache));
+  manager.setStatCache(std::move(statCache));
 
   // Create a virtual bar.cpp file.
   manager.getVirtualFile("bar.cpp", 200, 0);
@@ -260,7 +260,7 @@
   statCache->InjectDirectory("abc", 41);
   statCache->InjectFile("abc/foo.cpp", 42);
   statCache->InjectFile("abc/bar.cpp", 42);
-  manager.addStatCache(std::move(statCache));
+  manager.setStatCache(std::move(statCache));
 
   EXPECT_EQ(manager.getFile("abc/foo.cpp"), manager.getFile("abc/bar.cpp"));
 }
@@ -273,7 +273,7 @@
   statCache->InjectDirectory("abc", 41);
   statCache->InjectFile("abc/foo.cpp", 42);
   statCache->InjectFile("abc/bar.cpp", 42);
-  manager.addStatCache(std::move(statCache));
+  manager.setStatCache(std::move(statCache));
 
   ASSERT_TRUE(manager.getVirtualFile("abc/foo.cpp", 100, 0)->isValid());
   ASSERT_TRUE(manager.getVirtualFile("abc/bar.cpp", 200, 0)->isValid());
@@ -281,15 +281,6 @@
   EXPECT_EQ(manager.getFile("abc/foo.cpp"), manager.getFile("abc/bar.cpp"));
 }
 
-TEST_F(FileManagerTest, addRemoveStatCache) {
-  manager.addStatCache(llvm::make_unique());
-  auto statCacheOwner = llvm::make_unique();
-  auto *statCache = statCacheOwner.get();
-  manager.addStatCache(std::move(statCacheOwner));
-  manager.addStatCache(llvm::make_unique());
-  manager.removeStatCache(statCache);
-}
-
 // getFile() Should return the same entry as getVirtualFile if the file actually
 // is a virtual file, even if the name is not exactly the same (but is after
 // normalisation done by the file system, like on Windows). This can be checked
@@ -300,7 +291,7 @@
   statCache->InjectDirectory("c:\\tmp", 42);
   statCache->InjectFile("c:\\tmp\\test", 43);
 
-  manager.addStatCache(std::move(statCache));
+  manager.setStatCache(std::move(statCache));
 
   // Inject the virtual file:
   const FileEntry *file1 = manager.getVirtualFile("c:\\tmp\\test", 123, 1);
@@ -355,7 +346,7 @@
   auto statCache = llvm::make_unique();
   statCache->InjectDirectory("/tmp", 42);
   statCache->InjectFile("/tmp/test", 43);
-  manager.addStatCache(std::move(statCache));
+ 

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D55433#1323757 , @MyDeveloperDay 
wrote:

> a lot of projects aren't setup for c++17 yet which is needed for 
> [[nodiscard]] to be allowed,


You can use `[[clang::warn_unused_result]]` for this evaluation, that does not 
require C++17.

> But the clang-tidy -fix doesn't break the build, the patch is large but 
> nothing broke. (at compile time at least!)
> 
>   {F7661182} 

Uhm, so there wasn't a single true-positive in protobuf?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55433



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

[PATCH] D55443: [test] Capture stderr from 'tar --version' call as well

[Michał Górny via Phabricator via cfe-commits]

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



Comment at: test/lit.cfg.py:100
 config.available_features.add('gnutar')
-tar_version.wait()

ruiu wrote:
> Maybe a silly question, but is this OK to remove this line?
Yes. [[ 
https://docs.python.org/3/library/subprocess.html#subprocess.Popen.communicate 
| communicate() ]] waits on the process.


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

https://reviews.llvm.org/D55443



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

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I'm trying to generate an example of the modernize-use-nodiscard checker on 
something open source...as I developed this checker on Windows I struggled a 
little getting cmake to build me the json file to run clang-tidy over 
everything and a lot of projects aren't setup of c++17 yet which is needed for 
[[nodiscard]] to be allowed,  but using ClangPowerTools inside Visual studio I 
decided to run it on **protobuf**

I ran "Tidy Fix" with just the following custom checks 
"-*,modernize-use-nodiscard" this will fix some of the headers (alot of the 
headers actually)

recompiling the project, I see this one, which made me laugh...

  protobuf\protobuf\src\google\protobuf\descriptor.cc(4238): warning C4834: 
discarding return value of function with 'nodiscard' attribute

from descriptor.h I can see its fixed the header adding [[nodiscard]]'s

  // Tries to find something in the fallback database and link in the
  // corresponding proto file.  Returns true if successful, in which case
  // the caller should search for the thing again.  These are declared
  // const because they are called by (semantically) const methods.
  [[nodiscard]] bool TryFindFileInFallbackDatabase(const std::string& name) 
const;
  [[nodiscard]] bool TryFindSymbolInFallbackDatabase(const std::string& name) 
const;

ironically there is a comment in the code...

if (tables_->FindFile(proto.dependency(i)) == NULL &&
(pool_->underlay_ == NULL ||
 pool_->underlay_->FindFileByName(proto.dependency(i)) == NULL)) {
  // We don't care what this returns since we'll find out below anyway.
  pool_->TryFindFileInFallbackDatabase(proto.dependency(i));
  ^^
}
  }
  tables_->pending_files_.pop_back();

For me its as much about why the comment was put there, someone was having to 
explain why they didn't care about the return, but this could easily have been 
a bug

Bug the clang-tidy -fix doesn't break the build, the patch is large but nothing 
broke.

F7661182: protobuf.patch 


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55433



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

[PATCH] D55443: [test] Capture stderr from 'tar --version' call as well

[Rui Ueyama via Phabricator via cfe-commits]

ruiu added inline comments.



Comment at: test/lit.cfg.py:100
 config.available_features.add('gnutar')
-tar_version.wait()

Maybe a silly question, but is this OK to remove this line?


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

https://reviews.llvm.org/D55443



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

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:28-30
+bool isOperator(const FunctionDecl *D) {
+  return D->getNameAsString().find("operator") != std::string::npos;
+}

Can't you use `isOverloadedOperator()`?
No way going to `std::string` is fast.



Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:32-34
+bool isInternalFunction(const FunctionDecl *D) {
+  return D->getNameAsString().find("_") == 0;
+}

Motivation?
This should be configurable in some way.



Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:47
+void UseNodiscardCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus17)
+return;

This should be smarter, since you provide `ReplacementString`.
E.g. if `ReplacementString` is default, require C++17.
Else, only require C++.



Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:63-95
+  // if the location is invalid forget it
+  if (!MatchedDecl->getLocation().isValid())
+return;
+
+  if (MatchedDecl->isThisDeclarationADefinition() && 
MatchedDecl->isOutOfLine())
+return;
+

All this should be done in `registerMatchers()`.



Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:108
+  << MatchedDecl
+  << FixItHint::CreateInsertion(retLoc, NoDiscardMacro + " ");
+  return;

Can you actually provide fix-it though?
If the result will be unused, it will break build under `-Werror`.



Comment at: docs/ReleaseNotes.rst:79-81
+  Checks for non void const member functions which should have 
+  ``[[nodiscard]]`` added to tell the compiler a return type shoud not be 
+  ignored

The actual rules should be spelled out here, right now this sounds a bit too 
magical.



Comment at: docs/clang-tidy/checks/modernize-use-nodiscard.rst:6
+
+This check adds ``[[nodiscard]]`` attributes (introduced in C++17) to member 
functions to highlight at compile time where the return value of a function 
should not be ignored
+

Eugene.Zelenko wrote:
> Please use 80 characters limit.
Same, the rules need to be spelled out, this sounds a bit too magical right now,


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55433



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

[PATCH] D55451: [tests] Fix the FileManagerTest getVirtualFile test on Windows

[Stella Stamenova via Phabricator via cfe-commits]

stella.stamenova created this revision.
stella.stamenova added reviewers: kadircet, asmith.
Herald added a subscriber: cfe-commits.

The test passes on Windows only when it is executed on the C: drive. If the 
build and tests run on a different drive, the test is currently failing.


Repository:
  rC Clang

https://reviews.llvm.org/D55451

Files:
  unittests/Basic/FileManagerTest.cpp


Index: unittests/Basic/FileManagerTest.cpp
===
--- unittests/Basic/FileManagerTest.cpp
+++ unittests/Basic/FileManagerTest.cpp
@@ -351,22 +351,34 @@
 
 // getVirtualFile should always fill the real path.
 TEST_F(FileManagerTest, getVirtualFileFillsRealPathName) {
+  SmallString<64> CustomWorkingDir;
+#ifdef _WIN32
+  CustomWorkingDir = "C:/";
+#else
+  CustomWorkingDir = "/";
+#endif
+
+  auto FS = IntrusiveRefCntPtr(
+  new llvm::vfs::InMemoryFileSystem);
+  // setCurrentworkingdirectory must finish without error.
+  ASSERT_TRUE(!FS->setCurrentWorkingDirectory(CustomWorkingDir));
+
+  FileSystemOptions Opts;
+  FileManager Manager(Opts, FS);
+
   // Inject fake files into the file system.
   auto statCache = llvm::make_unique();
   statCache->InjectDirectory("/tmp", 42);
   statCache->InjectFile("/tmp/test", 43);
-  manager.addStatCache(std::move(statCache));
+
+  Manager.addStatCache(std::move(statCache));
 
   // Check for real path.
-  const FileEntry *file = manager.getVirtualFile("/tmp/test", 123, 1);
+  const FileEntry *file = Manager.getVirtualFile("/tmp/test", 123, 1);
   ASSERT_TRUE(file != nullptr);
   ASSERT_TRUE(file->isValid());
-  SmallString<64> ExpectedResult;
-#ifdef _WIN32
-  ExpectedResult = "C:/";
-#else
-  ExpectedResult = "/";
-#endif
+  SmallString<64> ExpectedResult = CustomWorkingDir;
+
   llvm::sys::path::append(ExpectedResult, "tmp", "test");
   EXPECT_EQ(file->tryGetRealPathName(), ExpectedResult);
 }


Index: unittests/Basic/FileManagerTest.cpp
===
--- unittests/Basic/FileManagerTest.cpp
+++ unittests/Basic/FileManagerTest.cpp
@@ -351,22 +351,34 @@
 
 // getVirtualFile should always fill the real path.
 TEST_F(FileManagerTest, getVirtualFileFillsRealPathName) {
+  SmallString<64> CustomWorkingDir;
+#ifdef _WIN32
+  CustomWorkingDir = "C:/";
+#else
+  CustomWorkingDir = "/";
+#endif
+
+  auto FS = IntrusiveRefCntPtr(
+  new llvm::vfs::InMemoryFileSystem);
+  // setCurrentworkingdirectory must finish without error.
+  ASSERT_TRUE(!FS->setCurrentWorkingDirectory(CustomWorkingDir));
+
+  FileSystemOptions Opts;
+  FileManager Manager(Opts, FS);
+
   // Inject fake files into the file system.
   auto statCache = llvm::make_unique();
   statCache->InjectDirectory("/tmp", 42);
   statCache->InjectFile("/tmp/test", 43);
-  manager.addStatCache(std::move(statCache));
+
+  Manager.addStatCache(std::move(statCache));
 
   // Check for real path.
-  const FileEntry *file = manager.getVirtualFile("/tmp/test", 123, 1);
+  const FileEntry *file = Manager.getVirtualFile("/tmp/test", 123, 1);
   ASSERT_TRUE(file != nullptr);
   ASSERT_TRUE(file->isValid());
-  SmallString<64> ExpectedResult;
-#ifdef _WIN32
-  ExpectedResult = "C:/";
-#else
-  ExpectedResult = "/";
-#endif
+  SmallString<64> ExpectedResult = CustomWorkingDir;
+
   llvm::sys::path::append(ExpectedResult, "tmp", "test");
   EXPECT_EQ(file->tryGetRealPathName(), ExpectedResult);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r348641 - [Preprocessor] Don't avoid entering included files after hitting a fatal error.

[Volodymyr Sapsai via cfe-commits]

Author: vsapsai
Date: Fri Dec  7 12:29:54 2018
New Revision: 348641

URL: http://llvm.org/viewvc/llvm-project?rev=348641&view=rev
Log:
[Preprocessor] Don't avoid entering included files after hitting a fatal error.

Change in r337953 violated the contract for `CXTranslationUnit_KeepGoing`:

> Do not stop processing when fatal errors are encountered.

Use different approach to fix long processing times with multiple inclusion
cycles. Instead of stopping preprocessing for fatal errors, do this after
reaching the max allowed include depth and only for the files that were
processed already. It is likely but not guaranteed those files cause a cycle.

rdar://problem/46108547

Reviewers: erik.pilkington, arphaman

Reviewed By: erik.pilkington

Subscribers: jkorous, dexonsmith, ilya-biryukov, Dmitry.Kozhevnikov

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


Added:
cfe/trunk/test/Index/Inputs/cycle.h
cfe/trunk/test/Index/keep-going-include-cycle.c
Modified:
cfe/trunk/include/clang/Lex/Preprocessor.h
cfe/trunk/lib/Lex/PPDirectives.cpp
cfe/trunk/test/Index/keep-going.cpp

Modified: cfe/trunk/include/clang/Lex/Preprocessor.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/Preprocessor.h?rev=348641&r1=348640&r2=348641&view=diff
==
--- cfe/trunk/include/clang/Lex/Preprocessor.h (original)
+++ cfe/trunk/include/clang/Lex/Preprocessor.h Fri Dec  7 12:29:54 2018
@@ -319,6 +319,10 @@ class Preprocessor {
   /// This is used when loading a precompiled preamble.
   std::pair SkipMainFilePreamble;
 
+  /// Whether we hit an error due to reaching max allowed include depth. Allows
+  /// to avoid hitting the same error over and over again.
+  bool HasReachedMaxIncludeDepth = false;
+
 public:
   struct PreambleSkipInfo {
 SourceLocation HashTokenLoc;

Modified: cfe/trunk/lib/Lex/PPDirectives.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=348641&r1=348640&r2=348641&view=diff
==
--- cfe/trunk/lib/Lex/PPDirectives.cpp (original)
+++ cfe/trunk/lib/Lex/PPDirectives.cpp Fri Dec  7 12:29:54 2018
@@ -1707,6 +1707,7 @@ void Preprocessor::HandleIncludeDirectiv
   // Check that we don't have infinite #include recursion.
   if (IncludeMacroStack.size() == MaxAllowedIncludeStackDepth-1) {
 Diag(FilenameTok, diag::err_pp_include_too_deep);
+HasReachedMaxIncludeDepth = true;
 return;
   }
 
@@ -1855,10 +1856,11 @@ void Preprocessor::HandleIncludeDirectiv
   if (PPOpts->SingleFileParseMode)
 ShouldEnter = false;
 
-  // Any diagnostics after the fatal error will not be visible. As the
-  // compilation failed already and errors in subsequently included files won't
-  // be visible, avoid preprocessing those files.
-  if (ShouldEnter && Diags->hasFatalErrorOccurred())
+  // If we've reached the max allowed include depth, it is usually due to an
+  // include cycle. Don't enter already processed files again as it can lead to
+  // reaching the max allowed include depth again.
+  if (ShouldEnter && HasReachedMaxIncludeDepth && File &&
+  HeaderInfo.getFileInfo(File).NumIncludes)
 ShouldEnter = false;
 
   // Determine whether we should try to import the module for this #include, if

Added: cfe/trunk/test/Index/Inputs/cycle.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Inputs/cycle.h?rev=348641&view=auto
==
--- cfe/trunk/test/Index/Inputs/cycle.h (added)
+++ cfe/trunk/test/Index/Inputs/cycle.h Fri Dec  7 12:29:54 2018
@@ -0,0 +1 @@
+#include "cycle.h"

Added: cfe/trunk/test/Index/keep-going-include-cycle.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/keep-going-include-cycle.c?rev=348641&view=auto
==
--- cfe/trunk/test/Index/keep-going-include-cycle.c (added)
+++ cfe/trunk/test/Index/keep-going-include-cycle.c Fri Dec  7 12:29:54 2018
@@ -0,0 +1,10 @@
+#include "cycle.h"
+#include "foo.h"
+
+// RUN: env CINDEXTEST_KEEP_GOING=1 c-index-test -test-print-type -I%S/Inputs 
%s 2> %t.stderr.txt | FileCheck %s
+// RUN: FileCheck -check-prefix CHECK-DIAG %s < %t.stderr.txt
+
+// Verify that we don't stop preprocessing after an include cycle.
+// CHECK: VarDecl=global_var:1:12 [type=int] [typekind=Int] [isPOD=1]
+
+// CHECK-DIAG: cycle.h:1:10: error: #include nested too deeply

Modified: cfe/trunk/test/Index/keep-going.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/keep-going.cpp?rev=348641&r1=348640&r2=348641&view=diff
==
--- cfe/trunk/test/Index/keep-going.cpp (original)
+++ cfe/trunk/test/Index/keep-going.cpp Fri Dec  7 12:29:54 2018
@@ -9,11 +9,18 @@ class B : public A { };
 
 class C : public A { };
 
-// R

[PATCH] D55385: [analyzer] RetainCountChecker: remove untested, unused, incorrect option IncludeAllocationLine

[George Karpenkov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC348637: [analyzer] RetainCountChecker: remove untested, 
unused, incorrect option… (authored by george.karpenkov, committed by ).
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D55385?vs=177055&id=177275#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D55385

Files:
  lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
  lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
  lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h


Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
===
--- lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
+++ lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
@@ -634,8 +634,7 @@
   UniqueingDecl = AllocNode->getLocationContext()->getDecl();
 }
 
-void CFRefLeakReport::createDescription(CheckerContext &Ctx,
-bool IncludeAllocationLine) {
+void CFRefLeakReport::createDescription(CheckerContext &Ctx) {
   assert(Location.isValid() && UniqueingDecl && UniqueingLocation.isValid());
   Description.clear();
   llvm::raw_string_ostream os(Description);
@@ -644,10 +643,6 @@
   Optional RegionDescription = describeRegion(AllocBinding);
   if (RegionDescription) {
 os << " stored into '" << *RegionDescription << '\'';
-if (IncludeAllocationLine) {
-  FullSourceLoc SL(AllocStmt->getBeginLoc(), Ctx.getSourceManager());
-  os << " (allocated on line " << SL.getSpellingLineNumber() << ")";
-}
   } else {
 
 // If we can't figure out the name, just supply the type information.
@@ -658,15 +653,14 @@
 CFRefLeakReport::CFRefLeakReport(CFRefBug &D, const LangOptions &LOpts,
  const SummaryLogTy &Log,
  ExplodedNode *n, SymbolRef sym,
- CheckerContext &Ctx,
- bool IncludeAllocationLine)
+ CheckerContext &Ctx)
   : CFRefReport(D, LOpts, Log, n, sym, false) {
 
   deriveAllocLocation(Ctx, sym);
   if (!AllocBinding)
 deriveParamLocation(Ctx, sym);
 
-  createDescription(Ctx, IncludeAllocationLine);
+  createDescription(Ctx);
 
   addVisitor(llvm::make_unique(sym, Log));
 }
Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
+++ lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
@@ -1059,8 +1059,7 @@
 if (N) {
   const LangOptions &LOpts = C.getASTContext().getLangOpts();
   auto R = llvm::make_unique(
-  *getLeakAtReturnBug(LOpts), LOpts, SummaryLog, N, Sym, C,
-  IncludeAllocationLine);
+  *getLeakAtReturnBug(LOpts), LOpts, SummaryLog, N, Sym, C);
   C.emitReport(std::move(R));
 }
 return N;
@@ -1346,7 +1345,7 @@
   assert(BT && "BugType not initialized.");
 
   Ctx.emitReport(llvm::make_unique(
-  *BT, LOpts, SummaryLog, N, *I, Ctx, IncludeAllocationLine));
+  *BT, LOpts, SummaryLog, N, *I, Ctx));
 }
   }
 
@@ -1508,8 +1507,6 @@
 
   AnalyzerOptions &Options = Mgr.getAnalyzerOptions();
 
-  Chk->IncludeAllocationLine = Options.getCheckerBooleanOption(
-   "leak-diagnostics-reference-allocation", false, 
Chk);
   Chk->ShouldCheckOSObjectRetainCount = Options.getCheckerBooleanOption(
 "CheckOSObject", true, 
Chk);
 }
Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h
===
--- lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h
+++ lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h
@@ -71,13 +71,12 @@
   // Finds the location where a leak warning for 'sym' should be raised.
   void deriveAllocLocation(CheckerContext &Ctx, SymbolRef sym);
   // Produces description of a leak warning which is printed on the console.
-  void createDescription(CheckerContext &Ctx, bool IncludeAllocationLine);
+  void createDescription(CheckerContext &Ctx);
 
 public:
   CFRefLeakReport(CFRefBug &D, const LangOptions &LOpts,
   const SummaryLogTy &Log, ExplodedNode *n, SymbolRef sym,
-  CheckerContext &Ctx,
-  bool IncludeAllocationLine);
+  CheckerContext &Ctx);
 
   PathDiagnosticLocation getLocation(const SourceManager &SM) const override {
 assert(Location.isValid());


Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/Retai

[PATCH] D55400: [analyzer] Move out tracking retain count for OSObjects into a separate checker

[George Karpenkov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC348638: [analyzer] Move out tracking retain count for 
OSObjects into a separate checker (authored by george.karpenkov, committed by ).
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D55400?vs=177271&id=177276#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D55400

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  include/clang/StaticAnalyzer/Core/RetainSummaryManager.h
  lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
  lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
  lib/StaticAnalyzer/Core/RetainSummaryManager.cpp
  test/Analysis/osobject-retain-release.cpp
  test/Analysis/test-separate-retaincount.cpp

Index: lib/StaticAnalyzer/Core/RetainSummaryManager.cpp
===
--- lib/StaticAnalyzer/Core/RetainSummaryManager.cpp
+++ lib/StaticAnalyzer/Core/RetainSummaryManager.cpp
@@ -24,6 +24,31 @@
 using namespace clang;
 using namespace ento;
 
+template 
+constexpr static bool isOneOf() {
+  return false;
+}
+
+/// Helper function to check whether the class is one of the
+/// rest of varargs.
+template 
+constexpr static bool isOneOf() {
+  return std::is_same::value || isOneOf();
+}
+
+template  bool RetainSummaryManager::isAttrEnabled() {
+  if (isOneOf()) {
+return TrackObjCAndCFObjects;
+  } else if (isOneOf()) {
+return TrackOSObjects;
+  }
+  llvm_unreachable("Unexpected attribute passed");
+}
+
 ArgEffects RetainSummaryManager::getArgEffects() {
   ArgEffects AE = ScratchArgs;
   ScratchArgs = AF.getEmptyMap();
@@ -116,30 +141,60 @@
 }
 
 const RetainSummary *
-RetainSummaryManager::generateSummary(const FunctionDecl *FD,
-  bool &AllowAnnotations) {
-  // We generate "stop" summaries for implicitly defined functions.
-  if (FD->isImplicit()) {
-return getPersistentStopSummary();
+RetainSummaryManager::getSummaryForOSObject(const FunctionDecl *FD,
+StringRef FName, QualType RetTy) {
+  if (RetTy->isPointerType()) {
+const CXXRecordDecl *PD = RetTy->getPointeeType()->getAsCXXRecordDecl();
+if (PD && isOSObjectSubclass(PD)) {
+  if (const IdentifierInfo *II = FD->getIdentifier()) {
+if (isOSObjectDynamicCast(II->getName()))
+  return getDefaultSummary();
+
+// All objects returned with functions *not* starting with
+// get, or iterators, are returned at +1.
+if ((!II->getName().startswith("get") &&
+ !II->getName().startswith("Get")) ||
+isOSIteratorSubclass(PD)) {
+  return getOSSummaryCreateRule(FD);
+} else {
+  return getOSSummaryGetRule(FD);
+}
+  }
+}
   }
 
-  const IdentifierInfo *II = FD->getIdentifier();
+  if (const auto *MD = dyn_cast(FD)) {
+const CXXRecordDecl *Parent = MD->getParent();
+if (TrackOSObjects && Parent && isOSObjectSubclass(Parent)) {
+  if (FName == "release")
+return getOSSummaryReleaseRule(FD);
 
-  StringRef FName = II ? II->getName() : "";
+  if (FName == "retain")
+return getOSSummaryRetainRule(FD);
 
-  // Strip away preceding '_'.  Doing this here will effect all the checks
-  // down below.
-  FName = FName.substr(FName.find_first_not_of('_'));
+  if (FName == "free")
+return getOSSummaryFreeRule(FD);
+
+  if (MD->getOverloadedOperator() == OO_New)
+return getOSSummaryCreateRule(MD);
+}
+  }
+
+  return nullptr;
+}
+
+const RetainSummary *RetainSummaryManager::getSummaryForObjCOrCFObject(
+const FunctionDecl *FD,
+StringRef FName,
+QualType RetTy,
+const FunctionType *FT,
+bool &AllowAnnotations) {
 
-  // Inspect the result type. Strip away any typedefs.
-  const auto *FT = FD->getType()->getAs();
-  QualType RetTy = FT->getReturnType();
   std::string RetTyName = RetTy.getAsString();
 
   // FIXME: This should all be refactored into a chain of "summary lookup"
   //  filters.
   assert(ScratchArgs.isEmpty());
-
   if (FName == "pthread_create" || FName == "pthread_setspecific") {
 // Part of:  and .
 // This will be addressed better with IPA.
@@ -230,30 +285,11 @@
 
   if (RetTy->isPointerType()) {
 
-const CXXRecordDecl *PD = RetTy->getPointeeType()->getAsCXXRecordDecl();
-if (TrackOSObjects && PD && isOSObjectSubclass(PD)) {
-  if (const IdentifierInfo *II = FD->getIdentifier()) {
-
-if (isOSObjectDynamicCast(II->getName()))
-  return getDefaultSummary();
-
-// All objects returned with functions *not* starting with
-// get, or iterators, are returned at +1.
-if ((!II->getName().startswith("get") &&
- !II->getName().startswith("Get")) ||
-isOSIteratorSubclass(PD)) {
-  retu

r348638 - [analyzer] Move out tracking retain count for OSObjects into a separate checker

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Fri Dec  7 12:21:51 2018
New Revision: 348638

URL: http://llvm.org/viewvc/llvm-project?rev=348638&view=rev
Log:
[analyzer] Move out tracking retain count for OSObjects into a separate checker

Allow enabling and disabling tracking of ObjC/CF objects
separately from tracking of OS objects.

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

Added:
cfe/trunk/test/Analysis/test-separate-retaincount.cpp
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h

cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp

cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp
cfe/trunk/test/Analysis/osobject-retain-release.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td?rev=348638&r1=348637&r2=348638&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td Fri Dec  7 
12:21:51 2018
@@ -520,6 +520,9 @@ def MacOSKeychainAPIChecker : Checker<"S
 def ObjCPropertyChecker : Checker<"ObjCProperty">,
   HelpText<"Check for proper uses of Objective-C properties">;
 
+def OSObjectRetainCountChecker : Checker<"OSObjectRetainCount">,
+  HelpText<"Check for leaks and improper reference count management for 
OSObject">;
+
 } // end "osx"
 
 let ParentPackage = Cocoa in {

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h?rev=348638&r1=348637&r2=348638&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h Fri Dec  
7 12:21:51 2018
@@ -469,6 +469,8 @@ public:
   }
 };
 
+class RetainSummaryTemplate;
+
 class RetainSummaryManager {
   typedef llvm::DenseMap
   FuncSummariesTy;
@@ -483,7 +485,10 @@ class RetainSummaryManager {
   /// Records whether or not the analyzed code runs in ARC mode.
   const bool ARCEnabled;
 
-  /// Track sublcasses of OSObject
+  /// Track Objective-C and CoreFoundation objects.
+  const bool TrackObjCAndCFObjects;
+
+  /// Track sublcasses of OSObject.
   const bool TrackOSObjects;
 
   /// FuncSummaries - A map from FunctionDecls to summaries.
@@ -626,13 +631,36 @@ class RetainSummaryManager {
   const RetainSummary * generateSummary(const FunctionDecl *FD,
 bool &AllowAnnotations);
 
+  /// Return a summary for OSObject, or nullptr if not found.
+  const RetainSummary *getSummaryForOSObject(const FunctionDecl *FD,
+ StringRef FName, QualType RetTy);
+
+  /// Return a summary for Objective-C or CF object, or nullptr if not found.
+  const RetainSummary *getSummaryForObjCOrCFObject(
+const FunctionDecl *FD,
+StringRef FName,
+QualType RetTy,
+const FunctionType *FT,
+bool &AllowAnnotations);
+
+  /// Apply the annotation of {@code pd} in function {@code FD}
+  /// to the resulting summary stored in out-parameter {@code Template}.
+  /// \return whether an annotation was applied.
+  bool applyFunctionParamAnnotationEffect(const ParmVarDecl *pd,
+unsigned parm_idx,
+const FunctionDecl *FD,
+ArgEffects::Factory &AF,
+RetainSummaryTemplate &Template);
+
 public:
   RetainSummaryManager(ASTContext &ctx,
bool usesARC,
-   bool trackOSObject)
+   bool trackObjCAndCFObjects,
+   bool trackOSObjects)
: Ctx(ctx),
  ARCEnabled(usesARC),
- TrackOSObjects(trackOSObject),
+ TrackObjCAndCFObjects(trackObjCAndCFObjects),
+ TrackOSObjects(trackOSObjects),
  AF(BPAlloc), ScratchArgs(AF.getEmptyMap()),
  ObjCAllocRetE(usesARC ? RetEffect::MakeNotOwned(RetEffect::ObjC)
: RetEffect::MakeOwned(RetEffect::ObjC)),
@@ -709,6 +737,7 @@ public:
   void updateSummaryFromAnnotations(const RetainSummary *&Summ,
 const FunctionDecl *FD);
 
+
   void updateSummaryForCall(const RetainSummary *&Summ,
 const CallEvent &Call);
 
@@ -716,9 +745,21 @@ public:
 
   RetEffect getObjAllocRetEffect() const { return ObjCAllocRetE; }
 
+  /// \return True if the declaration has an attribute {@code T},
+  /// AND we are

r348637 - [analyzer] RetainCountChecker: remove untested, unused, incorrect option IncludeAllocationLine

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Fri Dec  7 12:21:37 2018
New Revision: 348637

URL: http://llvm.org/viewvc/llvm-project?rev=348637&view=rev
Log:
[analyzer] RetainCountChecker: remove untested, unused, incorrect option 
IncludeAllocationLine

The option has no tests, is not used anywhere, and is actually
incorrect: it prints the line number without the reference to a file,
which can be outright incorrect.

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

Modified:

cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp

cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp

cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h

Modified: 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp?rev=348637&r1=348636&r2=348637&view=diff
==
--- 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp 
(original)
+++ 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp 
Fri Dec  7 12:21:37 2018
@@ -1059,8 +1059,7 @@ ExplodedNode * RetainCountChecker::check
 if (N) {
   const LangOptions &LOpts = C.getASTContext().getLangOpts();
   auto R = llvm::make_unique(
-  *getLeakAtReturnBug(LOpts), LOpts, SummaryLog, N, Sym, C,
-  IncludeAllocationLine);
+  *getLeakAtReturnBug(LOpts), LOpts, SummaryLog, N, Sym, C);
   C.emitReport(std::move(R));
 }
 return N;
@@ -1346,7 +1345,7 @@ RetainCountChecker::processLeaks(Program
   assert(BT && "BugType not initialized.");
 
   Ctx.emitReport(llvm::make_unique(
-  *BT, LOpts, SummaryLog, N, *I, Ctx, IncludeAllocationLine));
+  *BT, LOpts, SummaryLog, N, *I, Ctx));
 }
   }
 
@@ -1508,8 +1507,6 @@ void ento::registerRetainCountChecker(Ch
 
   AnalyzerOptions &Options = Mgr.getAnalyzerOptions();
 
-  Chk->IncludeAllocationLine = Options.getCheckerBooleanOption(
-   "leak-diagnostics-reference-allocation", false, 
Chk);
   Chk->ShouldCheckOSObjectRetainCount = Options.getCheckerBooleanOption(
 "CheckOSObject", true, 
Chk);
 }

Modified: 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp?rev=348637&r1=348636&r2=348637&view=diff
==
--- 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
 (original)
+++ 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
 Fri Dec  7 12:21:37 2018
@@ -634,8 +634,7 @@ void CFRefLeakReport::deriveAllocLocatio
   UniqueingDecl = AllocNode->getLocationContext()->getDecl();
 }
 
-void CFRefLeakReport::createDescription(CheckerContext &Ctx,
-bool IncludeAllocationLine) {
+void CFRefLeakReport::createDescription(CheckerContext &Ctx) {
   assert(Location.isValid() && UniqueingDecl && UniqueingLocation.isValid());
   Description.clear();
   llvm::raw_string_ostream os(Description);
@@ -644,10 +643,6 @@ void CFRefLeakReport::createDescription(
   Optional RegionDescription = describeRegion(AllocBinding);
   if (RegionDescription) {
 os << " stored into '" << *RegionDescription << '\'';
-if (IncludeAllocationLine) {
-  FullSourceLoc SL(AllocStmt->getBeginLoc(), Ctx.getSourceManager());
-  os << " (allocated on line " << SL.getSpellingLineNumber() << ")";
-}
   } else {
 
 // If we can't figure out the name, just supply the type information.
@@ -658,15 +653,14 @@ void CFRefLeakReport::createDescription(
 CFRefLeakReport::CFRefLeakReport(CFRefBug &D, const LangOptions &LOpts,
  const SummaryLogTy &Log,
  ExplodedNode *n, SymbolRef sym,
- CheckerContext &Ctx,
- bool IncludeAllocationLine)
+ CheckerContext &Ctx)
   : CFRefReport(D, LOpts, Log, n, sym, false) {
 
   deriveAllocLocation(Ctx, sym);
   if (!AllocBinding)
 deriveParamLocation(Ctx, sym);
 
-  createDescription(Ctx, IncludeAllocationLine);
+  createDescription(Ctx);
 
   addVisitor(llvm::make_unique(sym, Log));
 }

Modified: 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h?rev=348637&r1=348636&r2=348637&view=diff
==

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

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

Szelethus added a comment.

I recently had trouble with ignoring the return type, I would certainly 
appreciate a tool like this. Can you test it on LLVM+Clang? Maybe some methods 
in the AST library could also benefit from `LLVM_NODISCARD` (containers with 
factories is my no1 thought), and it's also a large enough codebase that it 
would be representative as to how the check behaves.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55433



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

[PATCH] D53830: [clang-tidy]: Abseil: new check 'abseil-upgrade-duration-conversions'

[Eric Fiselier via Phabricator via cfe-commits]

EricWF closed this revision.
EricWF added a comment.

Committed as r348633.


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

https://reviews.llvm.org/D53830



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

[PATCH] D55245: [clang-tidy] Add the abseil-duration-subtraction check

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang-tidy/abseil/DurationRewriter.cpp:35
+getInverseForScale(DurationScale Scale) {
+  static const std::unordered_map>

hwright wrote:
> lebedev.ri wrote:
> > https://llvm.org/docs/ProgrammersManual.html#other-map-like-container-options
> > > We never use hash_set and unordered_set because they are generally very 
> > > expensive (each insertion requires a malloc) and very non-portable.
> > 
> > Since the key is an enum, [[ 
> > https://llvm.org/docs/ProgrammersManual.html#llvm-adt-indexedmap-h | 
> > `llvm/ADT/IndexedMap.h` ]] should be a much better fit.
> It doesn't look like `IndexedMap` has a constructor which takes an 
> initializer list.  Without it, this code get a bit more difficult to read, 
> and I'd prefer to optimize for readability here.
The manual still 'recommends' not to use them.
Simple solution: immediately invoked lambda
Better solution: try to add such constructor to `IndexedMap`.



Comment at: clang-tidy/abseil/DurationRewriter.cpp:68
+  getInverseForScale(Scale);
+  if (const Expr *MaybeCallArg = selectFirst(
+  "e", match(callExpr(callee(functionDecl(

hwright wrote:
> lebedev.ri wrote:
> > lebedev.ri wrote:
> > > `if (const auto *MaybeCallArg`
> > Early return?
> I'm not quite sure what you mean by `Early return?`  Are you suggesting that 
> the call to `selectFirst` should be pulled out of the `if` conditional, and 
> then the inverse checked to return `llvm::None` first? 
Ah, nevermind then.



Comment at: clang-tidy/abseil/DurationRewriter.cpp:74
+ Node, *Result.Context))) {
+return tooling::fixit::getText(*MaybeCallArg, *Result.Context).str();
+  }

hwright wrote:
> lebedev.ri wrote:
> > So you generate fix-it, and then immediately degrade it into a string. 
> > Weird.
> This doesn't generate a fix-it, it just fetches the text of the given node as 
> a `StringRef` but we're returning a `string`, so we need to convert.
> 
> Is there a more canonical method I should use to fetch a node's text?
I don't know the answer, but have you tried looking what 
`tooling::fixit::getText()` does internally?



Comment at: clang-tidy/abseil/DurationRewriter.cpp:156
+llvm::Optional getScaleForInverse(llvm::StringRef Name) {
+  static const llvm::DenseMap ScaleMap(
+  {{"ToDoubleHours", DurationScale::Hours},

Are you very sure this shouldn't be [[ 
https://llvm.org/docs/ProgrammersManual.html#llvm-adt-stringmap-h | `StringMap` 
]]?


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

https://reviews.llvm.org/D55245



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

[clang-tools-extra] r348633 - [clang-tidy]: Abseil: new check 'abseil-upgrade-duration-conversions'

[Eric Fiselier via cfe-commits]

Author: ericwf
Date: Fri Dec  7 12:03:03 2018
New Revision: 348633

URL: http://llvm.org/viewvc/llvm-project?rev=348633&view=rev
Log:
[clang-tidy]: Abseil: new check 'abseil-upgrade-duration-conversions'

Patch by Alex Strelnikov.
Reviewed as D53830

Introduce a new check to upgrade user code based on upcoming API breaking 
changes to absl::Duration.

The check finds calls to arithmetic operators and factory functions for 
absl::Duration that rely on
an implicit user defined conversion to int64_t. These cases will no longer 
compile after proposed
changes are released. Suggested fixes explicitly cast the argument int64_t.

Added:

clang-tools-extra/trunk/clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp
clang-tools-extra/trunk/clang-tidy/abseil/UpgradeDurationConversionsCheck.h

clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-upgrade-duration-conversions.rst

clang-tools-extra/trunk/test/clang-tidy/abseil-upgrade-duration-conversions.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp?rev=348633&r1=348632&r2=348633&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp Fri Dec  7 
12:03:03 2018
@@ -20,6 +20,7 @@
 #include "RedundantStrcatCallsCheck.h"
 #include "StringFindStartswithCheck.h"
 #include "StrCatAppendCheck.h"
+#include "UpgradeDurationConversionsCheck.h"
 
 namespace clang {
 namespace tidy {
@@ -47,6 +48,8 @@ public:
 "abseil-str-cat-append");
 CheckFactories.registerCheck(
 "abseil-string-find-startswith");
+CheckFactories.registerCheck(
+"abseil-upgrade-duration-conversions");
   }
 };
 

Modified: clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt?rev=348633&r1=348632&r2=348633&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt Fri Dec  7 
12:03:03 2018
@@ -13,6 +13,7 @@ add_clang_library(clangTidyAbseilModule
   RedundantStrcatCallsCheck.cpp
   StrCatAppendCheck.cpp
   StringFindStartswithCheck.cpp
+  UpgradeDurationConversionsCheck.cpp
 
   LINK_LIBS
   clangAST

Added: 
clang-tools-extra/trunk/clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp?rev=348633&view=auto
==
--- 
clang-tools-extra/trunk/clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp 
(added)
+++ 
clang-tools-extra/trunk/clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp 
Fri Dec  7 12:03:03 2018
@@ -0,0 +1,158 @@
+//===--- UpgradeDurationConversionsCheck.cpp - clang-tidy 
-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "UpgradeDurationConversionsCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+void UpgradeDurationConversionsCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+return;
+
+  // For the arithmetic calls, we match only the uses of the templated 
operators
+  // where the template parameter is not a built-in type. This means the
+  // instantiation makes use of an available user defined conversion to
+  // `int64_t`.
+  //
+  // The implementation of these templates will be updated to fail SFINAE for
+  // non-integral types. We match them to suggest an explicit cast.
+
+  // Match expressions like `a *= b` and `a /= b` where `a` has type
+  // `absl::Duration` and `b` is not of a built-in type.
+  Finder->addMatcher(
+  cxxOperatorCallExpr(
+  argumentCountIs(2),
+  hasArgument(
+  0, expr(hasType(cxxRecordDecl(hasName("::absl::Duration"),
+  hasArgument(1, expr().bind("arg")),
+  callee(functionDecl(
+  hasParent(functionTemplateDecl()),
+  unless(hasTemplateArgument(0, refersToType(builtinType(,
+  hasAnyName("operator*=",

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

[Steve Canon via Phabricator via cfe-commits]

scanon added inline comments.



Comment at: 
libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp:187
+  assert(std::fabs(v[0] - 0.0f) < FLT_EPSILON);
+  assert(std::fabs(v[1] - 1.0f) < FLT_EPSILON);
+  assert(std::fabs(v[2] - 2.0f) < FLT_EPSILON);

These comparisons with `FLT_EPSILON` are doing nothing but adding noise. There 
is no value other than 2 that can possibly satisfy `fabs(v[2] - 2.0f) < 
FLT_EPSILON`, so this test can simply be `v[2] == 2`, for example.

If you find yourself using `FLT_EPSILON` as a tolerance, it's almost always 
wrong.


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

https://reviews.llvm.org/D48342



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

[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

[vit9696 via Phabricator via cfe-commits]

vit9696 added a comment.

In D49754#1321078 , @glaubitz wrote:

> I have applied this patch to the llvm-toolchain-7 package in Debian and did 
> not see any regressions on x86_64 or 32-Bit PowerPC. Additionally, I have 
> included the patches from https://reviews.llvm.org/D54409 and 
> https://reviews.llvm.org/D54583 saw no regressions on x86_64 and 32-bit 
> PowerPC.
>
> All three patches will be part of the next upload of the llvm-toolchain-7 
> package in Debian unstable which will be version 1:7.0.1~+rc2-9.


Might the first crash from https://reviews.llvm.org/D49754#1183753 reproduce 
for you or perhaps you have already bisected to trunk to figure out the 
changest that fixes it?


Repository:
  rC Clang

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

https://reviews.llvm.org/D49754



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

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

If you use some of my suggestions, please make sure it compiles in C++03 mode. 
I might be using some C++11 features (not sure whether default argument for 
template parameters is C++03).




Comment at: 
libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp:186
+  std::vector v(array, array + 3);
+  assert(std::fabs(v[0] - 0.0f) < FLT_EPSILON);
+  assert(std::fabs(v[1] - 1.0f) < FLT_EPSILON);

vsapsai wrote:
> ldionne wrote:
> > I do not understand this test, can you please explain?
> At some point I had implementation that was using memcpy for initialization 
> in this case. But in memory `{0, 1, 2} != {0.0f, 1.0f, 2.0f}`. I think I'll 
> change the test to initialize `vector` with `float[]` and it will 
> simplify asserts. Because currently those fabs and FLT_EPSILON are noisy and 
> add unnecessary cognitive load.
Ok.


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

https://reviews.llvm.org/D48342



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

[PATCH] D55245: [clang-tidy] Add the abseil-duration-subtraction check

[Hyrum Wright via Phabricator via cfe-commits]

hwright added inline comments.



Comment at: clang-tidy/abseil/DurationRewriter.cpp:35
+getInverseForScale(DurationScale Scale) {
+  static const std::unordered_map>

lebedev.ri wrote:
> https://llvm.org/docs/ProgrammersManual.html#other-map-like-container-options
> > We never use hash_set and unordered_set because they are generally very 
> > expensive (each insertion requires a malloc) and very non-portable.
> 
> Since the key is an enum, [[ 
> https://llvm.org/docs/ProgrammersManual.html#llvm-adt-indexedmap-h | 
> `llvm/ADT/IndexedMap.h` ]] should be a much better fit.
It doesn't look like `IndexedMap` has a constructor which takes an initializer 
list.  Without it, this code get a bit more difficult to read, and I'd prefer 
to optimize for readability here.



Comment at: clang-tidy/abseil/DurationRewriter.cpp:68
+  getInverseForScale(Scale);
+  if (const Expr *MaybeCallArg = selectFirst(
+  "e", match(callExpr(callee(functionDecl(

lebedev.ri wrote:
> lebedev.ri wrote:
> > `if (const auto *MaybeCallArg`
> Early return?
I'm not quite sure what you mean by `Early return?`  Are you suggesting that 
the call to `selectFirst` should be pulled out of the `if` conditional, and 
then the inverse checked to return `llvm::None` first? 



Comment at: clang-tidy/abseil/DurationRewriter.cpp:74
+ Node, *Result.Context))) {
+return tooling::fixit::getText(*MaybeCallArg, *Result.Context).str();
+  }

lebedev.ri wrote:
> So you generate fix-it, and then immediately degrade it into a string. Weird.
This doesn't generate a fix-it, it just fetches the text of the given node as a 
`StringRef` but we're returning a `string`, so we need to convert.

Is there a more canonical method I should use to fetch a node's text?


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

https://reviews.llvm.org/D55245



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

[PATCH] D55245: [clang-tidy] Add the abseil-duration-subtraction check

[Hyrum Wright via Phabricator via cfe-commits]

hwright updated this revision to Diff 177266.
hwright marked 8 inline comments as done.

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

https://reviews.llvm.org/D55245

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/DurationComparisonCheck.cpp
  clang-tidy/abseil/DurationRewriter.cpp
  clang-tidy/abseil/DurationRewriter.h
  clang-tidy/abseil/DurationSubtractionCheck.cpp
  clang-tidy/abseil/DurationSubtractionCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-duration-subtraction.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/Inputs/absl/time/time.h
  test/clang-tidy/abseil-duration-comparison.cpp
  test/clang-tidy/abseil-duration-factory-float.cpp
  test/clang-tidy/abseil-duration-factory-scale.cpp
  test/clang-tidy/abseil-duration-subtraction.cpp

Index: test/clang-tidy/abseil-duration-subtraction.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-duration-subtraction.cpp
@@ -0,0 +1,56 @@
+// RUN: %check_clang_tidy %s abseil-duration-subtraction %t -- -- -I %S/Inputs
+
+#include "absl/time/time.h"
+
+void f() {
+  double x;
+  absl::Duration d, d1;
+
+  x = absl::ToDoubleSeconds(d) - 1.0;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleSeconds(d - absl::Seconds(1))
+  x = absl::ToDoubleSeconds(d) - absl::ToDoubleSeconds(d1);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleSeconds(d - d1);
+  x = absl::ToDoubleSeconds(d) - 6.5 - 8.0;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleSeconds(d - absl::Seconds(6.5)) - 8.0;
+  x = absl::ToDoubleHours(d) - 1.0;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleHours(d - absl::Hours(1))
+  x = absl::ToDoubleMinutes(d) - 1;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleMinutes(d - absl::Minutes(1))
+  x = absl::ToDoubleMilliseconds(d) - 9;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleMilliseconds(d - absl::Milliseconds(9))
+  x = absl::ToDoubleMicroseconds(d) - 9;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleMicroseconds(d - absl::Microseconds(9))
+  x = absl::ToDoubleNanoseconds(d) - 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleNanoseconds(d - absl::Nanoseconds(42))
+
+  // We can rewrite the argument of the duration conversion
+#define THIRTY absl::Seconds(30)
+  x = absl::ToDoubleSeconds(THIRTY) - 1.0;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleSeconds(THIRTY - absl::Seconds(1))
+#undef THIRTY
+
+  // Some other contexts
+  if (absl::ToDoubleSeconds(d) - 1.0 > 10) {}
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: if (absl::ToDoubleSeconds(d - absl::Seconds(1)) > 10) {}
+
+  // These should not match
+  x = 5 - 6;
+  x = 4 - absl::ToDoubleSeconds(d) - 6.5 - 8.0;
+  x = absl::ToDoubleSeconds(d) + 1.0;
+  x = absl::ToDoubleSeconds(d) * 1.0;
+  x = absl::ToDoubleSeconds(d) / 1.0;
+
+#define MINUS_FIVE(z) absl::ToDoubleSeconds(z) - 5
+  x = MINUS_FIVE(d);
+#undef MINUS_FIVE
+}
Index: test/clang-tidy/abseil-duration-factory-scale.cpp
===
--- test/clang-tidy/abseil-duration-factory-scale.cpp
+++ test/clang-tidy/abseil-duration-factory-scale.cpp
@@ -1,32 +1,6 @@
-// RUN: %check_clang_tidy %s abseil-duration-factory-scale %t
+// RUN: %check_clang_tidy %s abseil-duration-factory-scale %t -- -- -I %S/Inputs
 
-// Mimic the implementation of absl::Duration
-namespace absl {
-
-class Duration {};
-
-Duration Nanoseconds(long long);
-Duration Microseconds(long long);
-Duration Milliseconds(long long);
-Duration Seconds(long long);
-Duration Minutes(long long);
-Duration Hours(long long);
-
-#define GENERATE_DURATION_FACTORY_OVERLOADS(NAME) \
-  Duration NAME(float n); \
-  Duration NAME(double n);\
-  template\
-  Duration NAME(T n);
-
-GENERATE_DURATION_FACTORY_OVERLOADS(Nanoseconds);
-GENERATE_DURATION_FACTORY_OVERLOADS(Microseconds);
-GENERATE_DURATION_F

[PATCH] D55443: [test] Capture stderr from 'tar --version' call as well

[Michał Górny via Phabricator via cfe-commits]

mgorny updated this revision to Diff 177265.
mgorny marked 2 inline comments as done.

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

https://reviews.llvm.org/D55443

Files:
  test/lit.cfg.py


Index: test/lit.cfg.py
===
--- test/lit.cfg.py
+++ test/lit.cfg.py
@@ -94,7 +94,10 @@
 tar_executable = lit.util.which('tar', config.environment['PATH'])
 if tar_executable:
 tar_version = subprocess.Popen(
-[tar_executable, '--version'], stdout=subprocess.PIPE, env={'LANG': 
'C'})
-if 'GNU tar' in tar_version.stdout.read().decode():
+[tar_executable, '--version'],
+stdout=subprocess.PIPE,
+stderr=subprocess.PIPE,
+env={'LANG': 'C'})
+sout, _ = tar_version.communicate()
+if 'GNU tar' in sout.decode():
 config.available_features.add('gnutar')
-tar_version.wait()


Index: test/lit.cfg.py
===
--- test/lit.cfg.py
+++ test/lit.cfg.py
@@ -94,7 +94,10 @@
 tar_executable = lit.util.which('tar', config.environment['PATH'])
 if tar_executable:
 tar_version = subprocess.Popen(
-[tar_executable, '--version'], stdout=subprocess.PIPE, env={'LANG': 'C'})
-if 'GNU tar' in tar_version.stdout.read().decode():
+[tar_executable, '--version'],
+stdout=subprocess.PIPE,
+stderr=subprocess.PIPE,
+env={'LANG': 'C'})
+sout, _ = tar_version.communicate()
+if 'GNU tar' in sout.decode():
 config.available_features.add('gnutar')
-tar_version.wait()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Thanks for the feedback. Answered one simple question, the rest needs more time.




Comment at: 
libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp:186
+  std::vector v(array, array + 3);
+  assert(std::fabs(v[0] - 0.0f) < FLT_EPSILON);
+  assert(std::fabs(v[1] - 1.0f) < FLT_EPSILON);

ldionne wrote:
> I do not understand this test, can you please explain?
At some point I had implementation that was using memcpy for initialization in 
this case. But in memory `{0, 1, 2} != {0.0f, 1.0f, 2.0f}`. I think I'll change 
the test to initialize `vector` with `float[]` and it will simplify 
asserts. Because currently those fabs and FLT_EPSILON are noisy and add 
unnecessary cognitive load.


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

https://reviews.llvm.org/D48342



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

[PATCH] D54604: Automatic variable initialization

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

I've addressed @pcc's comments. I'll now update the patch to mandate a 
`-Xclang` option for zero-init (otherwise it'll warn and pattern-init instead), 
and remove documentation for zero-init. I'm also thinking of changing float 
init to negative NaN with infinite scream payload.




Comment at: include/clang/Driver/ToolChain.h:355
+  virtual LangOptions::TrivialAutoVarInitKind
+  GetDefaultTrivialAutoVarInit() const {
+return LangOptions::TrivialAutoVarInitKind::Uninitialized;

pcc wrote:
> I wouldn't introduce this function because nothing is overriding it (for now, 
> at least).
I expect to do so for particular targets, so it's only a question of time, and 
for now it makes internal testing easier (I can build my compiler and override 
it for testing, before sending that upstream). I'd rather do it now, since it 
reduces merge conflicts as we kick the tires internally.



Comment at: lib/CodeGen/CGDecl.cpp:977
+  constexpr uint32_t SmallValue = 0x00AA;
+  if (Ty->isIntOrIntVectorTy()) {
+unsigned BitWidth = llvm::cast(

pcc wrote:
> Do you have test coverage for vectors?
`test/CodeGenCXX/auto-var-init.cpp` has those tests. I was lazy with this patch 
and hadn't updated the tests yet, in case people asked me to change the values. 
I've done so in this update.



Comment at: lib/CodeGen/CGDecl.cpp:1673
+llvm::BasicBlock *ContBB = createBasicBlock("vla-init.cont");
+EmitBlock(LoopBB);
+llvm::PHINode *Cur =

pcc wrote:
> I think we need to check that the size is non-zero here. Otherwise we're 
> going to clobber whatever comes after the VLA if its size is zero (I know 
> that C forbids zero-sized VLAs, but I believe we support them as a GNU 
> extension).
Indeed! Thanks for catching that. I thought I'd done it, and it turns out on O0 
it wasn't doing anything bad, and neither was it in O2 because the GEPs were 
marked inbounds so the codegen magically turned into a memset based on size!



Comment at: test/CodeGenCXX/trivial-auto-var-init.cpp:5
+
+// PATTERN-NOT: undef
+// ZERO-NOT: undef

pcc wrote:
> Since this is your only test involving structs and arrays, you probably want 
> to test them beyond the fact that they don't contain undef. I also believe 
> that this is testing for the absence of `undef` before the first label and 
> not the total absence of `undef` in the output.
I was lazy with testing in case the design changed significantly. The updated 
test explains what I was going for here :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D54604



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

[PATCH] D54604: Automatic variable initialization

[JF Bastien via Phabricator via cfe-commits]

jfb updated this revision to Diff 177262.
jfb marked 10 inline comments as done.
jfb added a comment.

- Make sure uninit-variables.c doesn't break.
- Address Peter's comments, improve tests.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54604

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/LangOptions.def
  include/clang/Basic/LangOptions.h
  include/clang/Driver/Options.td
  include/clang/Driver/ToolChain.h
  lib/CodeGen/CGDecl.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGenCXX/auto-var-init.cpp
  test/CodeGenCXX/trivial-auto-var-init-attribute.cpp
  test/CodeGenCXX/trivial-auto-var-init.cpp
  test/Sema/attr-trivial_auto_init.c
  test/Sema/uninit-variables.c

Index: test/Sema/uninit-variables.c
===
--- test/Sema/uninit-variables.c
+++ test/Sema/uninit-variables.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -Wuninitialized -Wconditional-uninitialized -fsyntax-only -fblocks %s -verify
+// RUN: %clang_cc1 -fsyntax-only -Wuninitialized -Wconditional-uninitialized -ftrivial-auto-var-init=pattern -fsyntax-only -fblocks %s -verify
 
 typedef __typeof(sizeof(int)) size_t;
 void *malloc(size_t);
Index: test/Sema/attr-trivial_auto_init.c
===
--- /dev/null
+++ test/Sema/attr-trivial_auto_init.c
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only
+
+void good() {
+int dont_initialize_me __attribute((trivial_auto_init("uninitialized")));
+int zero_me __attribute((trivial_auto_init("zero")));
+int pattern_me __attribute((trivial_auto_init("pattern")));
+}
+
+void bad() {
+int im_bad __attribute((trivial_auto_init)); // expected-error {{'trivial_auto_init' attribute takes one argument}}
+int im_baaad __attribute((trivial_auto_init("uninitialized", "zero"))); // expected-error {{'trivial_auto_init' attribute takes one argument}}
+static int come_on __attribute((trivial_auto_init("uninitialized"))); // expected-warning {{'trivial_auto_init' attribute only applies to local variables}}
+int you_know __attribute((trivial_auto_init("pony"))); // expected-warning {{'trivial_auto_init' attribute argument not supported: pony}}
+int and_the_whole_world_has_to __attribute((trivial_auto_init(uninitialized))); // expected-error {{'trivial_auto_init' attribute requires a string}}
+}
+
+extern int answer_right_now __attribute((trivial_auto_init("uninitialized"))); // expected-warning {{'trivial_auto_init' attribute only applies to local variables}}
+int just_to_tell_you_once_again __attribute((trivial_auto_init("uninitialized"))); // expected-warning {{'trivial_auto_init' attribute only applies to local variables}}
+static int whos_bad __attribute((trivial_auto_init("uninitialized"))); // expected-warning {{'trivial_auto_init' attribute only applies to local variables}}
+
+void the_word_is_out() __attribute((trivial_auto_init("uninitialized"))) {} // expected-warning {{'trivial_auto_init' attribute only applies to local variables}}
+void youre_doin_wrong(__attribute((trivial_auto_init("uninitialized"))) int a) {} // expected-warning {{'trivial_auto_init' attribute only applies to local variables}}
+
+struct GonnaLockYouUp {
+  __attribute((trivial_auto_init("uninitialized"))) int before_too_long; // expected-warning {{'trivial_auto_init' attribute only applies to local variables}}
+} __attribute((trivial_auto_init("uninitialized"))); // expected-warning {{'trivial_auto_init' attribute only applies to local variables}}
Index: test/CodeGenCXX/trivial-auto-var-init.cpp
===
--- /dev/null
+++ test/CodeGenCXX/trivial-auto-var-init.cpp
@@ -0,0 +1,216 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fblocks %s -emit-llvm -o - | FileCheck %s -check-prefix=UNINIT
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=pattern %s -emit-llvm -o - | FileCheck %s -check-prefix=PATTERN
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=zero %s -emit-llvm -o - | FileCheck %s -check-prefix=ZERO
+
+// None of the synthesized globals should contain `undef`.
+// PATTERN-NOT: undef
+// ZERO-NOT: undef
+
+template void used(T &) noexcept;
+
+extern "C" {
+
+// UNINIT-LABEL:  test_selfinit(
+// ZERO-LABEL:test_selfinit(
+// ZERO: store i32 0, i32* %self, align 4
+// PATTERN-LABEL: test_selfinit(
+// PATTERN: store i32 -1431655766, i32* %self, align 4
+void test_selfinit() {
+  int self = self + 1;
+  used(self);
+}
+
+// UNINIT-LABEL:  test_block(
+// ZERO-LABEL:test_block(
+// ZERO: store i32 0, i32* %block, align 4
+// PATTERN-LABEL: test_block(
+// PATTERN: store i32 -1431655766, i32* %block, align 4
+void test_block() {
+  __block int block;
+  used(block);

[PATCH] D55447: [Sema] Fix Modified Type in address_space AttributedType

[Leonard Chan via Phabricator via cfe-commits]

leonardchan created this revision.
leonardchan added reviewers: rsmith, aaron.ballman.
leonardchan added a project: clang.

This is a fix for https://reviews.llvm.org/D51229 where we pass the 
address_space qualified type as the modified type of an AttributedType. This 
change now instead wraps the AttributedType with either the address_space 
qualifier or a DependentAddressSpaceType.


Repository:
  rC Clang

https://reviews.llvm.org/D55447

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Sema/SemaType.cpp
  clang/tools/libclang/CXType.cpp

Index: clang/tools/libclang/CXType.cpp
===
--- clang/tools/libclang/CXType.cpp
+++ clang/tools/libclang/CXType.cpp
@@ -128,7 +128,8 @@
   if (TU && !T.isNull()) {
 // Handle attributed types as the original type
 if (auto *ATT = T->getAs()) {
-  if (!(TU->ParsingOptions & CXTranslationUnit_IncludeAttributedTypes)) {
+  if (!(TU->ParsingOptions & CXTranslationUnit_IncludeAttributedTypes) &&
+  ATT->getAttrKind() != attr::AddressSpace) {
 return MakeCXType(ATT->getModifiedType(), TU);
   }
 }
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -5740,28 +5740,26 @@
 // Type Attribute Processing
 //===--===//
 
-/// BuildAddressSpaceAttr - Builds a DependentAddressSpaceType if an expression
-/// is uninstantiated. If instantiated it will apply the appropriate address space
-/// to the type. This function allows dependent template variables to be used in
-/// conjunction with the address_space attribute
-QualType Sema::BuildAddressSpaceAttr(QualType &T, Expr *AddrSpace,
- SourceLocation AttrLoc) {
+/// Build an AddressSpace index from a constant expression and diagnose any
+/// errors related to invalid address_spaces. Returns true on successfully
+/// building an AddressSpace index.
+static bool BuildAddressSpaceIndex(LangAS &ASIdx, const Expr *AddrSpace,
+   SourceLocation AttrLoc, Sema &S) {
   if (!AddrSpace->isValueDependent()) {
-
 llvm::APSInt addrSpace(32);
-if (!AddrSpace->isIntegerConstantExpr(addrSpace, Context)) {
-  Diag(AttrLoc, diag::err_attribute_argument_type)
+if (!AddrSpace->isIntegerConstantExpr(addrSpace, S.Context)) {
+  S.Diag(AttrLoc, diag::err_attribute_argument_type)
   << "'address_space'" << AANT_ArgumentIntegerConstant
   << AddrSpace->getSourceRange();
-  return QualType();
+  return false;
 }
 
 // Bounds checking.
 if (addrSpace.isSigned()) {
   if (addrSpace.isNegative()) {
-Diag(AttrLoc, diag::err_attribute_address_space_negative)
+S.Diag(AttrLoc, diag::err_attribute_address_space_negative)
 << AddrSpace->getSourceRange();
-return QualType();
+return false;
   }
   addrSpace.setIsSigned(false);
 }
@@ -5770,14 +5768,28 @@
 max =
 Qualifiers::MaxAddressSpace - (unsigned)LangAS::FirstTargetAddressSpace;
 if (addrSpace > max) {
-  Diag(AttrLoc, diag::err_attribute_address_space_too_high)
+  S.Diag(AttrLoc, diag::err_attribute_address_space_too_high)
   << (unsigned)max.getZExtValue() << AddrSpace->getSourceRange();
-  return QualType();
+  return false;
 }
 
-LangAS ASIdx =
+ASIdx =
 getLangASFromTargetAS(static_cast(addrSpace.getZExtValue()));
+return true;
+  }
 
+  // Default value for DependentAddressSpaceTypes
+  ASIdx = LangAS::Default;
+  return true;
+}
+
+/// BuildAddressSpaceAttr - Builds a DependentAddressSpaceType if an expression
+/// is uninstantiated. If instantiated it will apply the appropriate address
+/// space to the type. This function allows dependent template variables to be
+/// used in conjunction with the address_space attribute
+QualType Sema::BuildAddressSpaceAttr(QualType &T, LangAS ASIdx, Expr *AddrSpace,
+ SourceLocation AttrLoc) {
+  if (!AddrSpace->isValueDependent()) {
 // If this type is already address space qualified with a different
 // address space, reject it.
 // ISO/IEC TR 18037 S5.3 (amending C99 6.7.3): "No type shall be qualified
@@ -5808,6 +5820,14 @@
   return Context.getDependentAddressSpaceType(T, AddrSpace, AttrLoc);
 }
 
+QualType Sema::BuildAddressSpaceAttr(QualType &T, Expr *AddrSpace,
+ SourceLocation AttrLoc) {
+  LangAS ASIdx;
+  if (!BuildAddressSpaceIndex(ASIdx, AddrSpace, AttrLoc, *this))
+return QualType();
+  return BuildAddressSpaceAttr(T, ASIdx, AddrSpace, AttrLoc);
+}
+
 /// HandleAddressSpaceTypeAttribute - Process an address_space attribute on the
 /// specified type.  The attribute contains 1 argument, the id of the address
 ///

[PATCH] D55443: [test] Capture stderr from 'tar --version' call as well

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: test/lit.cfg.py:101
+env={'LANG': 'C'})
+sout, serr = tar_version.communicate()
+if 'GNU tar' in sout.decode():

LG, but `serr` can be replaced by `_` if it is not used.


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

https://reviews.llvm.org/D55443



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

[PATCH] D55443: [test] Capture stderr from 'tar --version' call as well

[Rui Ueyama via Phabricator via cfe-commits]

ruiu added inline comments.



Comment at: test/lit.cfg.py:99
+stdout=subprocess.PIPE,
+stderr=subprocess.PIPE,
+env={'LANG': 'C'})

ruiu wrote:
> mgorny wrote:
> > MaskRay wrote:
> > > If you don't need stderr, remove `stderr=subprocess.PIPE,`
> > > 
> > > `subprocess.Popen followed by communicate()` can be replaced by 
> > > `check_output`
> > > 
> > > `if 'GNU tar' in sout:` does not in Python 3 as `sout` would have type 
> > > `bytes`, not `str`
> > I need to silence stderr. Given that `/dev/null` is not portable, and 
> > `subprocess.DEVNULL` requires py3.3+, capturing it is the simplest solution.
> > 
> > `check_output()` will throw when `tar` doesn't support version, i.e. it 
> > would break NetBSD completely instead of silencing the error.
> > 
> > I'll fix the type mismatch.
> I think as Fangrui suggested
> 
>   subprocess.check_output(["/tmp/foo"], stderr=subprocess.STDOUT)
> 
> is better than subprocess.Popen followed by communicate() and wait().
Oh sorry I didn't know that `check_output` throws if a subprocess exits with 
non-zero exit code.


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

https://reviews.llvm.org/D55443



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

[PATCH] D55443: [test] Capture stderr from 'tar --version' call as well

[Michał Górny via Phabricator via cfe-commits]

mgorny added a comment.

Sorry, it seems that my reply didn't go through.




Comment at: test/lit.cfg.py:99
+stdout=subprocess.PIPE,
+stderr=subprocess.PIPE,
+env={'LANG': 'C'})

MaskRay wrote:
> If you don't need stderr, remove `stderr=subprocess.PIPE,`
> 
> `subprocess.Popen followed by communicate()` can be replaced by `check_output`
> 
> `if 'GNU tar' in sout:` does not in Python 3 as `sout` would have type 
> `bytes`, not `str`
I need to silence stderr. Given that `/dev/null` is not portable, and 
`subprocess.DEVNULL` requires py3.3+, capturing it is the simplest solution.

`check_output()` will throw when `tar` doesn't support version, i.e. it would 
break NetBSD completely instead of silencing the error.

I'll fix the type mismatch.


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

https://reviews.llvm.org/D55443



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

[PATCH] D55443: [test] Capture stderr from 'tar --version' call as well

[Rui Ueyama via Phabricator via cfe-commits]

ruiu added inline comments.



Comment at: test/lit.cfg.py:99
+stdout=subprocess.PIPE,
+stderr=subprocess.PIPE,
+env={'LANG': 'C'})

MaskRay wrote:
> If you don't need stderr, remove `stderr=subprocess.PIPE,`
> 
> `subprocess.Popen followed by communicate()` can be replaced by `check_output`
> 
> `if 'GNU tar' in sout:` does not in Python 3 as `sout` would have type 
> `bytes`, not `str`
I think as Fangrui suggested

  subprocess.check_output(["/tmp/foo"], stderr=subprocess.STDOUT)

is better than subprocess.Popen followed by communicate() and wait().


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

https://reviews.llvm.org/D55443



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

[PATCH] D55441: [lld] [test] Fix reproduce-blackslash.s test with NetBSD tar

[Michał Górny via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL348628: [test] Fix reproduce-blackslash.s test with NetBSD 
tar (authored by mgorny, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D55441?vs=177232&id=177251#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55441

Files:
  lld/trunk/test/ELF/reproduce-backslash.s


Index: lld/trunk/test/ELF/reproduce-backslash.s
===
--- lld/trunk/test/ELF/reproduce-backslash.s
+++ lld/trunk/test/ELF/reproduce-backslash.s
@@ -6,4 +6,4 @@
 # RUN: ld.lld %T/foo\\.o --reproduce %T/repro.tar -o /dev/null
 # RUN: tar tf %T/repro.tar | FileCheck %s
 
-# CHECK: repro/{{.*}}/foo\\.o
+# CHECK: repro/{{.*}}/foo\{{[\]?}}.o


Index: lld/trunk/test/ELF/reproduce-backslash.s
===
--- lld/trunk/test/ELF/reproduce-backslash.s
+++ lld/trunk/test/ELF/reproduce-backslash.s
@@ -6,4 +6,4 @@
 # RUN: ld.lld %T/foo\\.o --reproduce %T/repro.tar -o /dev/null
 # RUN: tar tf %T/repro.tar | FileCheck %s
 
-# CHECK: repro/{{.*}}/foo\\.o
+# CHECK: repro/{{.*}}/foo\{{[\]?}}.o
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D55433#1323145 , @MyDeveloperDay 
wrote:

> In D55433#1323106 , @lebedev.ri 
> wrote:
>
> > Have you evaluated this on some major C++ projects yet?
> >  I suspect this may have pretty low SNR.
>
>
> Internally yes (on millions of lines), but you are correct high noise level,


... and the examples of the most common noise are?
Please run it on some codebase (llvm?) with run-clang-tidy.py, and post the 
results.

> however resulting compiles showed a number of hard to find bugs most notably 
> where empty()  function on custom container was being used when they meant 
> clear(), this bug had gone undetected for 10+ years.

That does sound good.

> The idea came from guidance given in Jason Turner @lefticus  CppCon2018 talk, 
> https://www.youtube.com/watch?v=DHOlsEd0eDE (about 14 minutes in)
> 
> "is it a logical error in your code to call this member function and discard 
> the return value"... the room says yes!!




Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55433



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

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:21
+namespace modernize {
+namespace {
+

Please use static instead anonymous namespace for functions. See LLVM code 
style guidelines.



Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:103
+
+  auto retLoc = MatchedDecl->getInnerLocStart();
+

Please don't use auto when type could not be deducted from same statement.



Comment at: docs/ReleaseNotes.rst:76
 
+- New :doc:`modernize-use-nodiscard
+  ` check.

Please use alphabetical order for new checks list.



Comment at: docs/clang-tidy/checks/modernize-use-nodiscard.rst:6
+
+This check adds ``[[nodiscard]]`` attributes (introduced in C++17) to member 
functions to highlight at compile time where the return value of a function 
should not be ignored
+

Please use 80 characters limit.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55433



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

[PATCH] D55443: [test] Capture stderr from 'tar --version' call as well

[Michał Górny via Phabricator via cfe-commits]

mgorny updated this revision to Diff 177250.
mgorny marked 2 inline comments as done.

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

https://reviews.llvm.org/D55443

Files:
  test/lit.cfg.py


Index: test/lit.cfg.py
===
--- test/lit.cfg.py
+++ test/lit.cfg.py
@@ -94,7 +94,10 @@
 tar_executable = lit.util.which('tar', config.environment['PATH'])
 if tar_executable:
 tar_version = subprocess.Popen(
-[tar_executable, '--version'], stdout=subprocess.PIPE, env={'LANG': 
'C'})
-if 'GNU tar' in tar_version.stdout.read().decode():
+[tar_executable, '--version'],
+stdout=subprocess.PIPE,
+stderr=subprocess.PIPE,
+env={'LANG': 'C'})
+sout, serr = tar_version.communicate()
+if 'GNU tar' in sout.decode():
 config.available_features.add('gnutar')
-tar_version.wait()


Index: test/lit.cfg.py
===
--- test/lit.cfg.py
+++ test/lit.cfg.py
@@ -94,7 +94,10 @@
 tar_executable = lit.util.which('tar', config.environment['PATH'])
 if tar_executable:
 tar_version = subprocess.Popen(
-[tar_executable, '--version'], stdout=subprocess.PIPE, env={'LANG': 'C'})
-if 'GNU tar' in tar_version.stdout.read().decode():
+[tar_executable, '--version'],
+stdout=subprocess.PIPE,
+stderr=subprocess.PIPE,
+env={'LANG': 'C'})
+sout, serr = tar_version.communicate()
+if 'GNU tar' in sout.decode():
 config.available_features.add('gnutar')
-tar_version.wait()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55410: [clang-tidy] check for flagging using declarations in headers

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: docs/clang-tidy/checks/abseil-alias-free-headers.rst:13
+The style guide  http://google.github.io/styleguide/cppguide.html#Aliases 
discusses this
+issue in more detail

Missing link to guidelines,


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55410



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

[PATCH] D55410: [clang-tidy] check for flagging using declarations in headers

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: docs/ReleaseNotes.rst:73
+
+  Flags using declarations in header files, and suggests that
+  these aliases be removed

Please highlight using with ``.



Comment at: docs/clang-tidy/checks/abseil-alias-free-headers.rst:6
+
+Flags using declarations in header files, and suggests that these aliases be 
removed.
+

Please highlight using with ``. Same in other places.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55410



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

[PATCH] D55441: [lld] [test] Fix reproduce-blackslash.s test with NetBSD tar

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

bsdtar is libarchive (by BSD people too)

We can align our native tar later and land this as is.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D55441



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

[PATCH] D34796: upporting -f(no)-reorder-functions flag, clang side change

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: docs/ClangCommandLineReference.rst:1953
 
+.. option:: -freorder-functions, -fno-reorder-functions
+

Isn't this autogenerated from `include/clang/Driver/Options.td`?


Repository:
  rC Clang

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

https://reviews.llvm.org/D34796



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

[PATCH] D55441: [lld] [test] Fix reproduce-blackslash.s test with NetBSD tar

[Rui Ueyama via Phabricator via cfe-commits]

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

LGTM

We might be able to change NetBSD's tar but I guess that takes very long time. 
This test is not very important in the sense that this tests just test a corner 
case. So I think we should just land this to make it work on NetBSD.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D55441



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

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

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

Szelethus added a comment.

In D54438#1315953 , @Szelethus wrote:

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


Pulling this off is not only difficult, certainly super-invasive, but also 
unnecessary -- in the final patch (D55429 ) I 
used a far simpler (~7 lines of code) solution, that still ensures that the 
checker naming problem never comes up again.

Thank you so much @NoQ for all the feedback! This project has been a super fun. 
I still expect some skeletons to fall out of the closet, but I'm fairly 
confident that the overall direction is set and is good.


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

https://reviews.llvm.org/D54438



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

[PATCH] D55441: [lld] [test] Fix reproduce-blackslash.s test with NetBSD tar

[Rui Ueyama via Phabricator via cfe-commits]

ruiu added a comment.

I thought NetBSD's tar is bsdtar because it's BSD... Anyways, I think I'm fine 
with this change, as the new test (which matches both `foo\\.o` and `foo\.o`) 
does not match a string that we don't want it to match.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D55441



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