[PATCH] D47393: [clang-format] Disable AlwaysBreakBeforeMultilineStrings in Google style for Objective-C 📜

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 148705.

Repository:
  rC Clang

https://reviews.llvm.org/D47393

Files:
  lib/Format/Format.cpp
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -1193,6 +1193,18 @@
"}");
 }
 
+TEST_F(FormatTestObjC, AlwaysBreakBeforeMultilineStrings) {
+  Style = getGoogleStyle(FormatStyle::LK_ObjC);
+  verifyFormat(" = @\"\"\n"
+   "   @\"\";");
+  verifyFormat("(@\"\"\n"
+   " @\"\");");
+  verifyFormat("(qqq, @\"\"\n"
+   "  @\"\");");
+  verifyFormat("NSString *const kString = @\"\"\n"
+   "  @\"\");");
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -799,6 +799,7 @@
 // has been implemented.
 GoogleStyle.BreakStringLiterals = false;
   } else if (Language == FormatStyle::LK_ObjC) {
+GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
 GoogleStyle.ColumnLimit = 100;
   }
 


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -1193,6 +1193,18 @@
"}");
 }
 
+TEST_F(FormatTestObjC, AlwaysBreakBeforeMultilineStrings) {
+  Style = getGoogleStyle(FormatStyle::LK_ObjC);
+  verifyFormat(" = @\"\"\n"
+   "   @\"\";");
+  verifyFormat("(@\"\"\n"
+   " @\"\");");
+  verifyFormat("(qqq, @\"\"\n"
+   "  @\"\");");
+  verifyFormat("NSString *const kString = @\"\"\n"
+   "  @\"\");");
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -799,6 +799,7 @@
 // has been implemented.
 GoogleStyle.BreakStringLiterals = false;
   } else if (Language == FormatStyle::LK_ObjC) {
+GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
 GoogleStyle.ColumnLimit = 100;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47340: [ClangDiagnostics] Silence warning about fallthrough after PrintFatalError

[Dávid Bolvanský via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL40: [ClangDiagnostics] Silence warning about fallthrough 
after PrintFatalError (authored by xbolva00, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D47340?vs=148523&id=148708#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D47340

Files:
  cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp


Index: cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp
===
--- cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp
+++ cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp
@@ -633,7 +633,7 @@
 return It->second.Root;
   }
 
-  void PrintFatalError(llvm::Twine const &Msg) const {
+  LLVM_ATTRIBUTE_NORETURN void PrintFatalError(llvm::Twine const &Msg) const {
 assert(EvaluatingRecord && "not evaluating a record?");
 llvm::PrintFatalError(EvaluatingRecord->getLoc(), Msg);
   }


Index: cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp
===
--- cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp
+++ cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp
@@ -633,7 +633,7 @@
 return It->second.Root;
   }
 
-  void PrintFatalError(llvm::Twine const &Msg) const {
+  LLVM_ATTRIBUTE_NORETURN void PrintFatalError(llvm::Twine const &Msg) const {
 assert(EvaluatingRecord && "not evaluating a record?");
 llvm::PrintFatalError(EvaluatingRecord->getLoc(), Msg);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47340: [ClangDiagnostics] Silence warning about fallthrough after PrintFatalError

[Dávid Bolvanský via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC40: [ClangDiagnostics] Silence warning about fallthrough 
after PrintFatalError (authored by xbolva00, committed by ).

Repository:
  rL LLVM

https://reviews.llvm.org/D47340

Files:
  utils/TableGen/ClangDiagnosticsEmitter.cpp


Index: utils/TableGen/ClangDiagnosticsEmitter.cpp
===
--- utils/TableGen/ClangDiagnosticsEmitter.cpp
+++ utils/TableGen/ClangDiagnosticsEmitter.cpp
@@ -633,7 +633,7 @@
 return It->second.Root;
   }
 
-  void PrintFatalError(llvm::Twine const &Msg) const {
+  LLVM_ATTRIBUTE_NORETURN void PrintFatalError(llvm::Twine const &Msg) const {
 assert(EvaluatingRecord && "not evaluating a record?");
 llvm::PrintFatalError(EvaluatingRecord->getLoc(), Msg);
   }


Index: utils/TableGen/ClangDiagnosticsEmitter.cpp
===
--- utils/TableGen/ClangDiagnosticsEmitter.cpp
+++ utils/TableGen/ClangDiagnosticsEmitter.cpp
@@ -633,7 +633,7 @@
 return It->second.Root;
   }
 
-  void PrintFatalError(llvm::Twine const &Msg) const {
+  LLVM_ATTRIBUTE_NORETURN void PrintFatalError(llvm::Twine const &Msg) const {
 assert(EvaluatingRecord && "not evaluating a record?");
 llvm::PrintFatalError(EvaluatingRecord->getLoc(), Msg);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46052: GNUstep Objective-C ABI version 2

[David Chisnall via Phabricator via cfe-commits]

theraven added inline comments.



Comment at: lib/CodeGen/CGObjCGNU.cpp:512
   /// used to return an untyped selector (with the types field set to NULL).
-  llvm::Value *GetSelector(CodeGenFunction &CGF, Selector Sel,
+  virtual llvm::Value *GetSelector(CodeGenFunction &CGF, Selector Sel,
const std::string &TypeEncoding);

xbolva00 wrote:
> This causes 
> CGObjCGNU.cpp:589:16: warning: ‘virtual llvm::Value* 
> {anonymous}::CGObjCGNU::GetSelector(clang::CodeGen::CodeGenFunction&, const 
> clang::ObjCMethodDecl*)’ was hidden [-Woverloaded-virtual]
>llvm::Value *GetSelector(CodeGenFunction &CGF,
> 
I can't reproduce this, and I'm not sure what the issue is.  The two lines have 
different overloads, so one shouldn't be hiding the other.


Repository:
  rC Clang

https://reviews.llvm.org/D46052



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

[PATCH] D46052: GNUstep Objective-C ABI version 2

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added inline comments.



Comment at: lib/CodeGen/CGObjCGNU.cpp:512
   /// used to return an untyped selector (with the types field set to NULL).
-  llvm::Value *GetSelector(CodeGenFunction &CGF, Selector Sel,
+  virtual llvm::Value *GetSelector(CodeGenFunction &CGF, Selector Sel,
const std::string &TypeEncoding);

theraven wrote:
> xbolva00 wrote:
> > This causes 
> > CGObjCGNU.cpp:589:16: warning: ‘virtual llvm::Value* 
> > {anonymous}::CGObjCGNU::GetSelector(clang::CodeGen::CodeGenFunction&, const 
> > clang::ObjCMethodDecl*)’ was hidden [-Woverloaded-virtual]
> >llvm::Value *GetSelector(CodeGenFunction &CGF,
> > 
> I can't reproduce this, and I'm not sure what the issue is.  The two lines 
> have different overloads, so one shouldn't be hiding the other.
It is a warning from GCC 7.2


Repository:
  rC Clang

https://reviews.llvm.org/D46052



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

[PATCH] D46052: GNUstep Objective-C ABI version 2

[David Chisnall via Phabricator via cfe-commits]

theraven added inline comments.



Comment at: lib/CodeGen/CGObjCGNU.cpp:512
   /// used to return an untyped selector (with the types field set to NULL).
-  llvm::Value *GetSelector(CodeGenFunction &CGF, Selector Sel,
+  virtual llvm::Value *GetSelector(CodeGenFunction &CGF, Selector Sel,
const std::string &TypeEncoding);

xbolva00 wrote:
> theraven wrote:
> > xbolva00 wrote:
> > > This causes 
> > > CGObjCGNU.cpp:589:16: warning: ‘virtual llvm::Value* 
> > > {anonymous}::CGObjCGNU::GetSelector(clang::CodeGen::CodeGenFunction&, 
> > > const clang::ObjCMethodDecl*)’ was hidden [-Woverloaded-virtual]
> > >llvm::Value *GetSelector(CodeGenFunction &CGF,
> > > 
> > I can't reproduce this, and I'm not sure what the issue is.  The two lines 
> > have different overloads, so one shouldn't be hiding the other.
> It is a warning from GCC 7.2
Sounds like it's a spurious one.  Any idea how to silence it?


Repository:
  rC Clang

https://reviews.llvm.org/D46052



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

[PATCH] D46052: GNUstep Objective-C ABI version 2

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added inline comments.



Comment at: lib/CodeGen/CGObjCGNU.cpp:512
   /// used to return an untyped selector (with the types field set to NULL).
-  llvm::Value *GetSelector(CodeGenFunction &CGF, Selector Sel,
+  virtual llvm::Value *GetSelector(CodeGenFunction &CGF, Selector Sel,
const std::string &TypeEncoding);

theraven wrote:
> xbolva00 wrote:
> > theraven wrote:
> > > xbolva00 wrote:
> > > > This causes 
> > > > CGObjCGNU.cpp:589:16: warning: ‘virtual llvm::Value* 
> > > > {anonymous}::CGObjCGNU::GetSelector(clang::CodeGen::CodeGenFunction&, 
> > > > const clang::ObjCMethodDecl*)’ was hidden [-Woverloaded-virtual]
> > > >llvm::Value *GetSelector(CodeGenFunction &CGF,
> > > > 
> > > I can't reproduce this, and I'm not sure what the issue is.  The two 
> > > lines have different overloads, so one shouldn't be hiding the other.
> > It is a warning from GCC 7.2
> Sounds like it's a spurious one.  Any idea how to silence it?
Not sure ..

virtual llvm::Value *GetSelector(CodeGenFunction &CGF, Selector Sel, const 
std::string &TypeEncoding); is here

but base class (CGObjCRuntime) has:

virtual llvm::Value *   GetSelector (CodeGenFunction &CGF, Selector Sel)=0


Repository:
  rC Clang

https://reviews.llvm.org/D46052



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

[PATCH] D44826: Add -Wunused-using, a warning that finds unused using declarations.

[Carlos Alberto Enciso via Phabricator via cfe-commits]

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

@lebedev.ri,

Thanks very much for your review. I will address those issues and update the 
patch.


https://reviews.llvm.org/D44826



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

[PATCH] D47135: [analyzer] A checker for dangling internal buffer pointers in C++

[Reka Kovacs via Phabricator via cfe-commits]

rnkovacs updated this revision to Diff 148727.
rnkovacs retitled this revision from "[analyzer][WIP] A checker for dangling 
string pointers in C++" to "[analyzer] A checker for dangling internal buffer 
pointers in C++".
rnkovacs edited the summary of this revision.
rnkovacs added a comment.

- All `basic_string` types are now supported.
- Mock tests added.
- New `AllocationFamily` `AF_InternalBuffer` introduced.
- NewDeleteChecker dependency added.


https://reviews.llvm.org/D47135

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/AllocationState.h
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  test/Analysis/dangling-internal-buffer.cpp

Index: test/Analysis/dangling-internal-buffer.cpp
===
--- /dev/null
+++ test/Analysis/dangling-internal-buffer.cpp
@@ -0,0 +1,62 @@
+//RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.DanglingInternalBuffer %s -verify
+
+namespace std {
+
+template< typename CharT >
+class basic_string {
+public:
+  ~basic_string();
+  const CharT *c_str();
+};
+
+typedef basic_string string;
+typedef basic_string wstring;
+typedef basic_string u16string;
+typedef basic_string u32string;
+
+} // end namespace std
+
+void consume(const char *) {}
+void consume(const wchar_t *) {}
+void consume(const char16_t *) {}
+void consume(const char32_t *) {}
+
+void deref_after_scope_char() {
+  const char *c;
+  {
+std::string s;
+c = s.c_str();
+  }
+  consume(c); // expected-warning {{Use of memory after it is freed}}
+  // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_scope_wchar_t() {
+  const wchar_t *w;
+  {
+std::wstring ws;
+w = ws.c_str();
+  }
+  consume(w); // expected-warning {{Use of memory after it is freed}}
+  // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_scope_char16_t() {
+  const char16_t *c16;
+  {
+std::u16string s16;
+c16 = s16.c_str();
+  }
+  consume(c16); // expected-warning {{Use of memory after it is freed}}
+  // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_scope_char32_t() {
+  const char32_t *c32;
+  {
+std::u32string s32;
+c32 = s32.c_str();
+  }
+  consume(c32); // expected-warning {{Use of memory after it is freed}}
+  // expected-note@-1 {{Use of memory after it is freed}}
+}
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -30,6 +30,7 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
+#include "AllocationState.h"
 #include 
 #include 
 
@@ -45,7 +46,8 @@
   AF_CXXNew,
   AF_CXXNewArray,
   AF_IfNameIndex,
-  AF_Alloca
+  AF_Alloca,
+  AF_InternalBuffer
 };
 
 class RefState {
@@ -1467,6 +1469,7 @@
 case AF_CXXNew: os << "'new'"; return;
 case AF_CXXNewArray: os << "'new[]'"; return;
 case AF_IfNameIndex: os << "'if_nameindex()'"; return;
+case AF_InternalBuffer: os << "container-specific allocator"; return;
 case AF_Alloca:
 case AF_None: llvm_unreachable("not a deallocation expression");
   }
@@ -1479,6 +1482,7 @@
 case AF_CXXNew: os << "'delete'"; return;
 case AF_CXXNewArray: os << "'delete[]'"; return;
 case AF_IfNameIndex: os << "'if_freenameindex()'"; return;
+case AF_InternalBuffer: os << "container-specific deallocator"; return;
 case AF_Alloca:
 case AF_None: llvm_unreachable("suspicious argument");
   }
@@ -1653,7 +1657,8 @@
 return Optional();
   }
   case AF_CXXNew:
-  case AF_CXXNewArray: {
+  case AF_CXXNewArray:
+  case AF_InternalBuffer: {
 if (IsALeakCheck) {
   if (ChecksEnabled[CK_NewDeleteLeaksChecker])
 return CK_NewDeleteLeaksChecker;
@@ -2991,6 +2996,20 @@
   }
 }
 
+namespace clang {
+namespace ento {
+namespace allocation_state {
+
+ProgramStateRef
+markReleased(ProgramStateRef State, SymbolRef Sym, const Expr *Origin) {
+  AllocationFamily Family = AF_InternalBuffer;
+  return State->set(Sym, RefState::getReleased(Family, Origin));
+}
+
+} // end namespace allocation_state
+} // end namespace ento
+} // end namespace clang
+
 void ento::registerNewDeleteLeaksChecker(CheckerManager &mgr) {
   registerCStringCheckerBasic(mgr);
   MallocChecker *checker = mgr.registerChecker();
Index: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
===
--- /dev/null
+++ lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
@@ -0,0 +1,85 @@
+//=== DanglingInternalBufferChecker.cpp ---*- C++ -*--//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University o

[PATCH] D47135: [analyzer] A checker for dangling internal buffer pointers in C++

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

Looks good so far, some comments inline.




Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:58
+
+  auto *TypeDecl = TypedR->getValueType().getTypePtr()->getAsCXXRecordDecl();
+  if (TypeDecl->getName() != "basic_string")

QualType should have overloaded `->` operator, I think you can remove the 
`getTypePtr`.



Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:65
+  if (Call.isCalled(CStrFn)) {
+SymbolRef RawPtr = Call.getReturnValue().getAsSymbol();
+State = State->set(TypedR, RawPtr);

I wonder if we can always get a symbol.
I can think of two cases when the call above could fail:
* Non-standard implementation that does not return a pointer
* The analyzer able to inline stuff and the returned value is a constant (a 
specific address that is shared between all empty strings in some 
implementation?)

Even though I do find any of the above likely. @NoQ what do you think? Does 
this worth a check?



Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:73
+if (State->contains(TypedR)) {
+  const SymbolRef *StrBufferPtr = State->get(TypedR);
+  const Expr *Origin = Call.getOriginExpr();

What if no symbol is associated with the region? Won't this return null that we 
dereference later on?



Comment at: test/Analysis/dangling-internal-buffer.cpp:24
+
+void deref_after_scope_char() {
+  const char *c;

I would like to see test cases that does not trigger warning.


https://reviews.llvm.org/D47135



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

[PATCH] D46805: If some platforms do not support an attribute, we should exclude the platform

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

@rsmith -- do the object file formats listed look correct to you?




Comment at: include/clang/Basic/Attr.td:322
+def TargetSupportsAlias : TargetSpec {
+  let ObjectFormats = ["COFF", "ELF", "Wasm"];
+}

Did you verify that Wasm supports the alias attribute? If it is supported, it 
might be nice to add a test to `CodeGen/alias.c` to demonstrate it. Similar for 
COFF.



Comment at: test/Sema/attr-alias-has.c:5
+// RUN: %clang_cc1 -triple wasm32-unknown-unknown -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple wasm64-unknown-unknown -fsyntax-only -verify %s
+

I'd like to see a test that the "attribute not supported on target" diagnostic 
is being generated. I'd recommend something along these lines:
```
void g() {}
void f() __attribute__((alias("g")));
#if !__has_attribute(alias)
// expected-error@-2{{expected diagnostic text}}
#else
// expected-no-diagnostics
#endif
```


Repository:
  rC Clang

https://reviews.llvm.org/D46805



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

[PATCH] D47135: [analyzer] A checker for dangling internal buffer pointers in C++

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:73
+if (State->contains(TypedR)) {
+  const SymbolRef *StrBufferPtr = State->get(TypedR);
+  const Expr *Origin = Call.getOriginExpr();

xazax.hun wrote:
> What if no symbol is associated with the region? Won't this return null that 
> we dereference later on?
Oh, never mind this one, I did not notice the `contains` call above.


https://reviews.llvm.org/D47135



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

[PATCH] D47135: [analyzer] A checker for dangling internal buffer pointers in C++

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:65
+  if (Call.isCalled(CStrFn)) {
+SymbolRef RawPtr = Call.getReturnValue().getAsSymbol();
+State = State->set(TypedR, RawPtr);

xazax.hun wrote:
> I wonder if we can always get a symbol.
> I can think of two cases when the call above could fail:
> * Non-standard implementation that does not return a pointer
> * The analyzer able to inline stuff and the returned value is a constant (a 
> specific address that is shared between all empty strings in some 
> implementation?)
> 
> Even though I do find any of the above likely. @NoQ what do you think? Does 
> this worth a check?
Sorry for the spam. Unfortunately, it is not possible to edit the comments.
I do *not* find any of the above likely.


https://reviews.llvm.org/D47135



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

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

This is relaxing `-Wformat` and making users instead write `-Wformat-pedantic` 
to get the strong guarantees, which is the opposite of what I thought the 
consensus was. Have I misunderstood something?




Comment at: include/clang/Analysis/Analyses/FormatString.h:263-265
+  ArgType(Kind k = UnknownTy, const char *n = nullptr) : K(k), Name(n) {}
+  ArgType(QualType t, const char *n = nullptr) : K(SpecificTy), T(t), Name(n) 
{}
+  ArgType(CanQualType t) : K(SpecificTy), T(t) {}

If we're going to update this, can you fix the parameter names to be in line 
with the coding conventions?



Comment at: lib/Sema/SemaChecking.cpp:6597-6599
+  const analyze_printf::ArgType::MatchKind match =
+  AT.matchesType(S.Context, ExprTy);
+  bool pedantic = match == analyze_printf::ArgType::NoMatchPedantic;

These identifiers also need some love to match the coding conventions. Happens 
elsewhere in this patch as well.


Repository:
  rC Clang

https://reviews.llvm.org/D47290



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

[PATCH] D47135: [analyzer] A checker for dangling internal buffer pointers in C++

[Reka Kovacs via Phabricator via cfe-commits]

rnkovacs updated this revision to Diff 148732.
rnkovacs added a comment.

Address (most) comments.


https://reviews.llvm.org/D47135

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/AllocationState.h
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  test/Analysis/dangling-internal-buffer.cpp

Index: test/Analysis/dangling-internal-buffer.cpp
===
--- /dev/null
+++ test/Analysis/dangling-internal-buffer.cpp
@@ -0,0 +1,71 @@
+//RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.DanglingInternalBuffer %s -analyzer-output=text -verify
+
+namespace std {
+
+template< typename CharT >
+class basic_string {
+public:
+  ~basic_string();
+  const CharT *c_str();
+};
+
+typedef basic_string string;
+typedef basic_string wstring;
+typedef basic_string u16string;
+typedef basic_string u32string;
+
+} // end namespace std
+
+void consume(const char *) {}
+void consume(const wchar_t *) {}
+void consume(const char16_t *) {}
+void consume(const char32_t *) {}
+
+void deref_after_scope_char() {
+  const char *c;
+  {
+std::string s;
+c = s.c_str();
+  }
+  consume(c); // expected-warning {{Use of memory after it is freed}}
+  // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_scope_wchar_t() {
+  const wchar_t *w;
+  {
+std::wstring ws;
+w = ws.c_str();
+  }
+  consume(w); // expected-warning {{Use of memory after it is freed}}
+  // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_scope_char16_t() {
+  const char16_t *c16;
+  {
+std::u16string s16;
+c16 = s16.c_str();
+  }
+  consume(c16); // expected-warning {{Use of memory after it is freed}}
+  // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_scope_char32_t() {
+  const char32_t *c32;
+  {
+std::u32string s32;
+c32 = s32.c_str();
+  }
+  consume(c32); // expected-warning {{Use of memory after it is freed}}
+  // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_scope_ok() {
+  const char *c;
+  std::string s;
+  {
+c = s.c_str();
+  }
+  consume(c); // no-warning
+}
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -30,6 +30,7 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
+#include "AllocationState.h"
 #include 
 #include 
 
@@ -45,7 +46,8 @@
   AF_CXXNew,
   AF_CXXNewArray,
   AF_IfNameIndex,
-  AF_Alloca
+  AF_Alloca,
+  AF_InternalBuffer
 };
 
 class RefState {
@@ -1467,6 +1469,7 @@
 case AF_CXXNew: os << "'new'"; return;
 case AF_CXXNewArray: os << "'new[]'"; return;
 case AF_IfNameIndex: os << "'if_nameindex()'"; return;
+case AF_InternalBuffer: os << "container-specific allocator"; return;
 case AF_Alloca:
 case AF_None: llvm_unreachable("not a deallocation expression");
   }
@@ -1479,6 +1482,7 @@
 case AF_CXXNew: os << "'delete'"; return;
 case AF_CXXNewArray: os << "'delete[]'"; return;
 case AF_IfNameIndex: os << "'if_freenameindex()'"; return;
+case AF_InternalBuffer: os << "container-specific deallocator"; return;
 case AF_Alloca:
 case AF_None: llvm_unreachable("suspicious argument");
   }
@@ -1653,7 +1657,8 @@
 return Optional();
   }
   case AF_CXXNew:
-  case AF_CXXNewArray: {
+  case AF_CXXNewArray:
+  case AF_InternalBuffer: {
 if (IsALeakCheck) {
   if (ChecksEnabled[CK_NewDeleteLeaksChecker])
 return CK_NewDeleteLeaksChecker;
@@ -2991,6 +2996,20 @@
   }
 }
 
+namespace clang {
+namespace ento {
+namespace allocation_state {
+
+ProgramStateRef
+markReleased(ProgramStateRef State, SymbolRef Sym, const Expr *Origin) {
+  AllocationFamily Family = AF_InternalBuffer;
+  return State->set(Sym, RefState::getReleased(Family, Origin));
+}
+
+} // end namespace allocation_state
+} // end namespace ento
+} // end namespace clang
+
 void ento::registerNewDeleteLeaksChecker(CheckerManager &mgr) {
   registerCStringCheckerBasic(mgr);
   MallocChecker *checker = mgr.registerChecker();
Index: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
===
--- /dev/null
+++ lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
@@ -0,0 +1,85 @@
+//=== DanglingInternalBufferChecker.cpp ---*- C++ -*--//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file defines a check that marks a raw p

[PATCH] D47416: [analyzer] Clean up the program state map of DanglingInternalBufferChecker

[Reka Kovacs via Phabricator via cfe-commits]

rnkovacs created this revision.
rnkovacs added reviewers: NoQ, xazax.hun, george.karpenkov.
Herald added subscribers: a.sidorin, dkrupp, szepet, baloghadamsoftware, 
whisperity.

Symbols are cleaned up from the program state map when they go out of scope. 
(This will need to be done individually when the collection of multiple symbols 
for one region will be supported.)
Regions are cleaned up when the corresponding object is destroyed.


Repository:
  rC Clang

https://reviews.llvm.org/D47416

Files:
  lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp


Index: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
+++ lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
@@ -26,7 +26,8 @@
 
 namespace {
 
-class DanglingInternalBufferChecker : public Checker {
+class DanglingInternalBufferChecker : public Checker {
   CallDescription CStrFn;
 
 public:
@@ -36,6 +37,9 @@
   /// corresponding string object region in the ProgramState. Mark the symbol
   /// released if the string object is destroyed.
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
+
+  /// Clean up the ProgramState map.
+  void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
 };
 
 } // end anonymous namespace
@@ -73,12 +77,28 @@
   const SymbolRef *StrBufferPtr = State->get(TypedR);
   const Expr *Origin = Call.getOriginExpr();
   State = allocation_state::markReleased(State, *StrBufferPtr, Origin);
+  State = State->remove(TypedR);
   C.addTransition(State);
   return;
 }
   }
 }
 
+void DanglingInternalBufferChecker::checkDeadSymbols(SymbolReaper &SymReaper,
+ CheckerContext &C) const {
+  if (!SymReaper.hasDeadSymbols())
+return;
+
+  ProgramStateRef State = C.getState();
+  RawPtrMapTy RPM = State->get();
+  for (const auto Region : RPM) {
+if (SymReaper.isDead(Region.second))
+  State = State->remove(Region.first);
+  }
+
+  C.addTransition(State);
+}
+
 void ento::registerDanglingInternalBufferChecker(CheckerManager &Mgr) {
   registerNewDeleteChecker(Mgr);
   Mgr.registerChecker();


Index: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
+++ lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
@@ -26,7 +26,8 @@
 
 namespace {
 
-class DanglingInternalBufferChecker : public Checker {
+class DanglingInternalBufferChecker : public Checker {
   CallDescription CStrFn;
 
 public:
@@ -36,6 +37,9 @@
   /// corresponding string object region in the ProgramState. Mark the symbol
   /// released if the string object is destroyed.
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
+
+  /// Clean up the ProgramState map.
+  void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
 };
 
 } // end anonymous namespace
@@ -73,12 +77,28 @@
   const SymbolRef *StrBufferPtr = State->get(TypedR);
   const Expr *Origin = Call.getOriginExpr();
   State = allocation_state::markReleased(State, *StrBufferPtr, Origin);
+  State = State->remove(TypedR);
   C.addTransition(State);
   return;
 }
   }
 }
 
+void DanglingInternalBufferChecker::checkDeadSymbols(SymbolReaper &SymReaper,
+ CheckerContext &C) const {
+  if (!SymReaper.hasDeadSymbols())
+return;
+
+  ProgramStateRef State = C.getState();
+  RawPtrMapTy RPM = State->get();
+  for (const auto Region : RPM) {
+if (SymReaper.isDead(Region.second))
+  State = State->remove(Region.first);
+  }
+
+  C.addTransition(State);
+}
+
 void ento::registerDanglingInternalBufferChecker(CheckerManager &Mgr) {
   registerNewDeleteChecker(Mgr);
   Mgr.registerChecker();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47157: Warning for framework headers using double quote includes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D47157#1107210, @bruno wrote:

> > See also PR22165.
>
> Nice, seems related to this indeed. Are you aware of any development along 
> those lines in clang-tidy? We would like this to be part of clang and be used 
> as part of the normal compiler workflow, it can surely be as well part of any 
> clang-tidy related include guidelines in the future.
>
> >> The warning is off by default.
> > 
> > We typically do not add off-by-default warnings because experience has 
> > shown the rarely get enabled in practice. Can you explain a bit more about 
> > why this one is off by default?
>
> Right. I believe this is going to be used in practice, the reason I'm adding 
> it involves some user demand for such warning. Such quoted include use in 
> frameworks happen often and we would like a smooth transition to happen here 
> (e.g. do not initially affect -Werror users). If it proves worth it, we can 
> migrate to on by default in the future. It wouldn't be a problem if we have 
> it on by default on open source and disable by default downstream, but I 
> rather be consistent.


Consistency would be nice, but at the same time, I don't see a good metric for 
when we'd know it's time to switch it to being on by default. I'm worried that 
it'll remain off by default forever simply because no one thinks to go turn it 
on (because it's silent by default). Perhaps on-by-default here and 
off-by-default downstream would be the better approach, or do you think this 
would be too disruptive to enable by default anywhere?




Comment at: include/clang/Basic/DiagnosticLexKinds.td:713
+def warn_quoted_include_in_framework_header : Warning<
+  "double-quoted include \"%0\" in framework header, expected system style 
 include"
+  >, InGroup, DefaultIgnore;

80-col limit?

Also, I'd probably drop "system style" and reword slightly to:

`"double-quoted include \"%0\" in framework header, expected angle-bracketed 
include <%0> instead"`



Comment at: lib/Lex/HeaderSearch.cpp:753-754
+  IncluderAndDir.second->getName()))
+Diags.Report(IncludeLoc,
+ diag::warn_quoted_include_in_framework_header)
+<< Filename;

This seems like a good place for a fix-it to switch the include style. Is there 
a reason to not do that work for the user?



Comment at: lib/Lex/HeaderSearch.cpp:873-874
+!FoundByHeaderMap)
+  Diags.Report(IncludeLoc, diag::warn_quoted_include_in_framework_header)
+  << Filename;
+

Similar here.


Repository:
  rC Clang

https://reviews.llvm.org/D47157



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

[PATCH] D47135: [analyzer] A checker for dangling internal buffer pointers in C++

[Gábor Horváth via Phabricator via cfe-commits]

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

LGTM!


https://reviews.llvm.org/D47135



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

r333348 - [X86] Remove mask from avx512ifma builtins. Use a select instruction instead.

[Craig Topper via cfe-commits]

Author: ctopper
Date: Sat May 26 11:55:26 2018
New Revision: 48

URL: http://llvm.org/viewvc/llvm-project?rev=48&view=rev
Log:
[X86] Remove mask from avx512ifma builtins. Use a select instruction instead.

This reduces from 12 builtins to 6 since we no longer need a mask and maskz 
version.

Modified:
cfe/trunk/include/clang/Basic/BuiltinsX86.def
cfe/trunk/lib/Headers/avx512ifmaintrin.h
cfe/trunk/lib/Headers/avx512ifmavlintrin.h
cfe/trunk/test/CodeGen/avx512ifma-builtins.c
cfe/trunk/test/CodeGen/avx512ifmavl-builtins.c

Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=48&r1=47&r2=48&view=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Sat May 26 11:55:26 2018
@@ -1461,18 +1461,12 @@ TARGET_BUILTIN(__builtin_ia32_movdqa64lo
 TARGET_BUILTIN(__builtin_ia32_movdqa64load256_mask, "V4LLiV4LLiC*V4LLiUc", 
"n", "avx512vl")
 TARGET_BUILTIN(__builtin_ia32_movdqa64store128_mask, "vV2LLi*V2LLiUc", "n", 
"avx512f")
 TARGET_BUILTIN(__builtin_ia32_movdqa64store256_mask, "vV4LLi*V4LLiUc", "n", 
"avx512f")
-TARGET_BUILTIN(__builtin_ia32_vpmadd52huq512_mask, "V8LLiV8LLiV8LLiV8LLiUc", 
"nc", "avx512ifma")
-TARGET_BUILTIN(__builtin_ia32_vpmadd52huq512_maskz, "V8LLiV8LLiV8LLiV8LLiUc", 
"nc", "avx512ifma")
-TARGET_BUILTIN(__builtin_ia32_vpmadd52luq512_mask, "V8LLiV8LLiV8LLiV8LLiUc", 
"nc", "avx512ifma")
-TARGET_BUILTIN(__builtin_ia32_vpmadd52luq512_maskz, "V8LLiV8LLiV8LLiV8LLiUc", 
"nc", "avx512ifma")
-TARGET_BUILTIN(__builtin_ia32_vpmadd52huq128_mask, "V2LLiV2LLiV2LLiV2LLiUc", 
"nc", "avx512ifma,avx512vl")
-TARGET_BUILTIN(__builtin_ia32_vpmadd52huq128_maskz, "V2LLiV2LLiV2LLiV2LLiUc", 
"nc", "avx512ifma,avx512vl")
-TARGET_BUILTIN(__builtin_ia32_vpmadd52huq256_mask, "V4LLiV4LLiV4LLiV4LLiUc", 
"nc", "avx512ifma,avx512vl")
-TARGET_BUILTIN(__builtin_ia32_vpmadd52huq256_maskz, "V4LLiV4LLiV4LLiV4LLiUc", 
"nc", "avx512ifma,avx512vl")
-TARGET_BUILTIN(__builtin_ia32_vpmadd52luq128_mask, "V2LLiV2LLiV2LLiV2LLiUc", 
"nc", "avx512ifma,avx512vl")
-TARGET_BUILTIN(__builtin_ia32_vpmadd52luq128_maskz, "V2LLiV2LLiV2LLiV2LLiUc", 
"nc", "avx512ifma,avx512vl")
-TARGET_BUILTIN(__builtin_ia32_vpmadd52luq256_mask, "V4LLiV4LLiV4LLiV4LLiUc", 
"nc", "avx512ifma,avx512vl")
-TARGET_BUILTIN(__builtin_ia32_vpmadd52luq256_maskz, "V4LLiV4LLiV4LLiV4LLiUc", 
"nc", "avx512ifma,avx512vl")
+TARGET_BUILTIN(__builtin_ia32_vpmadd52huq512, "V8LLiV8LLiV8LLiV8LLi", "nc", 
"avx512ifma")
+TARGET_BUILTIN(__builtin_ia32_vpmadd52luq512, "V8LLiV8LLiV8LLiV8LLi", "nc", 
"avx512ifma")
+TARGET_BUILTIN(__builtin_ia32_vpmadd52huq128, "V2LLiV2LLiV2LLiV2LLi", "nc", 
"avx512ifma,avx512vl")
+TARGET_BUILTIN(__builtin_ia32_vpmadd52huq256, "V4LLiV4LLiV4LLiV4LLi", "nc", 
"avx512ifma,avx512vl")
+TARGET_BUILTIN(__builtin_ia32_vpmadd52luq128, "V2LLiV2LLiV2LLiV2LLi", "nc", 
"avx512ifma,avx512vl")
+TARGET_BUILTIN(__builtin_ia32_vpmadd52luq256, "V4LLiV4LLiV4LLiV4LLi", "nc", 
"avx512ifma,avx512vl")
 TARGET_BUILTIN(__builtin_ia32_vpermi2varqi512_mask, "V64cV64cV64cV64cULLi", 
"nc", "avx512vbmi")
 TARGET_BUILTIN(__builtin_ia32_vpermt2varqi512_mask, "V64cV64cV64cV64cULLi", 
"nc", "avx512vbmi")
 TARGET_BUILTIN(__builtin_ia32_vpermt2varqi512_maskz, "V64cV64cV64cV64cULLi", 
"nc", "avx512vbmi")

Modified: cfe/trunk/lib/Headers/avx512ifmaintrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/avx512ifmaintrin.h?rev=48&r1=47&r2=48&view=diff
==
--- cfe/trunk/lib/Headers/avx512ifmaintrin.h (original)
+++ cfe/trunk/lib/Headers/avx512ifmaintrin.h Sat May 26 11:55:26 2018
@@ -34,57 +34,47 @@
 static __inline__ __m512i __DEFAULT_FN_ATTRS
 _mm512_madd52hi_epu64 (__m512i __X, __m512i __Y, __m512i __Z)
 {
-  return (__m512i) __builtin_ia32_vpmadd52huq512_mask ((__v8di) __X,
-   (__v8di) __Y,
-   (__v8di) __Z,
-   (__mmask8) -1);
+  return (__m512i)__builtin_ia32_vpmadd52huq512((__v8di) __X, (__v8di) __Y,
+(__v8di) __Z);
 }
 
 static __inline__ __m512i __DEFAULT_FN_ATTRS
-_mm512_mask_madd52hi_epu64 (__m512i __W, __mmask8 __M, __m512i __X,
-  __m512i __Y)
+_mm512_mask_madd52hi_epu64 (__m512i __W, __mmask8 __M, __m512i __X, __m512i 
__Y)
 {
-  return (__m512i) __builtin_ia32_vpmadd52huq512_mask ((__v8di) __W,
-   (__v8di) __X,
-   (__v8di) __Y,
-   (__mmask8) __M);
+  return (__m512i)__builtin_ia32_selectq_512(__M,
+   (__v8di)_mm512_madd52hi_epu64(__W, __X, 
__Y),
+   (__v8di)__W);
 }
 
 static __inline__ __m512i __DEFAULT_FN_ATTRS
 _mm512_maskz_madd52hi_epu64 (__mmask8 __M, __m512i __X, __m512i __Y, __m512i 
__Z)
 {
-  r

[PATCH] D47416: [analyzer] Clean up the program state map of DanglingInternalBufferChecker

[Gábor Horváth via Phabricator via cfe-commits]

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

LG!


Repository:
  rC Clang

https://reviews.llvm.org/D47416



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

[PATCH] D47135: [analyzer] A checker for dangling internal buffer pointers in C++

[Reka Kovacs via Phabricator via cfe-commits]

rnkovacs added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1661
+  case AF_CXXNewArray:
+  case AF_InternalBuffer: {
 if (IsALeakCheck) {

Is tying this new family to NewDeleteChecker reasonable? I did it because it 
was NewDelete that warned naturally before creating the new `AllocationFamily`. 
Should I introduce a new checker kind in MallocChecker for this purpose instead?


https://reviews.llvm.org/D47135



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

[PATCH] D47417: [analyzer] Add missing state transition in IteratorChecker

[Reka Kovacs via Phabricator via cfe-commits]

rnkovacs created this revision.
rnkovacs added reviewers: NoQ, xazax.hun, george.karpenkov, baloghadamsoftware.
Herald added subscribers: a.sidorin, dkrupp, szepet, whisperity.

After cleaning up program state maps in `checkDeadSymbols()`, a transition 
should be added to generate the new state.


Repository:
  rC Clang

https://reviews.llvm.org/D47417

Files:
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp


Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -395,6 +395,8 @@
   State = State->remove(Comp.first);
 }
   }
+
+  C.addTransition(State);
 }
 
 ProgramStateRef IteratorChecker::evalAssume(ProgramStateRef State, SVal Cond,


Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -395,6 +395,8 @@
   State = State->remove(Comp.first);
 }
   }
+
+  C.addTransition(State);
 }
 
 ProgramStateRef IteratorChecker::evalAssume(ProgramStateRef State, SVal Cond,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[JF Bastien via Phabricator via cfe-commits]

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

In https://reviews.llvm.org/D47290#1113365, @aaron.ballman wrote:

> This is relaxing `-Wformat` and making users instead write 
> `-Wformat-pedantic` to get the strong guarantees, which is the opposite of 
> what I thought the consensus was. Have I misunderstood something?


From Shoaib's email:

> Based on all the discussion so far, I think the consensus is that we don't 
> want to relax -Wformat by default, but an additional relaxed option would be 
> fine. That works for me (where I can just switch my codebases to use the new 
> warning option), but it wouldn't handle JF Bastien's use case, so he would 
> still want to pursue the relaxations specific to Apple platforms.

This patch is specific to the Darwin platform, as proposed in Alex' original 
patch and as I promised to do in the cfe-dev thread. Shoaib's follow-up would 
be different and do what I think you're referring to.


Repository:
  rC Clang

https://reviews.llvm.org/D47290



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

[PATCH] D45686: [Driver] Clean up tmp files when deleting Compilation objects

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D45686#1109295, @dstenb wrote:

> Ping.
>
> We have added a lit reproducer for this now in clang-tools-extra: 
> https://reviews.llvm.org/D47251.


The above should be rolled into this patch as the test case verifying the 
behavioral changes.


https://reviews.llvm.org/D45686



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

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[JF Bastien via Phabricator via cfe-commits]

jfb updated this revision to Diff 148737.
jfb marked 2 inline comments as done.
jfb added a comment.

- Fix variable capitalization.


Repository:
  rC Clang

https://reviews.llvm.org/D47290

Files:
  include/clang/Analysis/Analyses/FormatString.h
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Analysis/PrintfFormatString.cpp
  lib/Sema/SemaChecking.cpp
  test/FixIt/fixit-format-ios-nopedantic.m
  test/FixIt/fixit-format-ios.m
  test/SemaObjC/format-size-spec-nsinteger.m

Index: test/SemaObjC/format-size-spec-nsinteger.m
===
--- /dev/null
+++ test/SemaObjC/format-size-spec-nsinteger.m
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -triple thumbv7-apple-ios -Wno-objc-root-class -fsyntax-only -verify -Wformat %s
+// RUN: %clang_cc1 -triple thumbv7-apple-ios -Wno-objc-root-class -fsyntax-only -verify -Wformat-pedantic -DPEDANTIC %s
+
+#if !defined(PEDANTIC)
+// expected-no-diagnostics
+#endif
+
+#if __LP64__
+typedef unsigned long NSUInteger;
+typedef long NSInteger;
+#else
+typedef unsigned int NSUInteger;
+typedef int NSInteger;
+#endif
+
+@class NSString;
+
+extern void NSLog(NSString *format, ...);
+
+void testSizeSpecifier() {
+  NSInteger i = 0;
+  NSUInteger j = 0;
+  NSLog(@"max NSInteger = %zi", i);
+  NSLog(@"max NSUinteger = %zu", j);
+
+#if defined(PEDANTIC)
+  // expected-warning@-4 {{values of type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead}}
+  // expected-warning@-4 {{values of type 'NSUInteger' should not be used as format arguments; add an explicit cast to 'unsigned long' instead}}
+#endif
+}
Index: test/FixIt/fixit-format-ios.m
===
--- test/FixIt/fixit-format-ios.m
+++ test/FixIt/fixit-format-ios.m
@@ -1,5 +1,5 @@
 // RUN: cp %s %t
-// RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -fsyntax-only -Wformat -fixit %t
+// RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -fsyntax-only -Wformat-pedantic -fixit %t
 // RUN: grep -v CHECK %t | FileCheck %s
 
 int printf(const char * restrict, ...);
Index: test/FixIt/fixit-format-ios-nopedantic.m
===
--- /dev/null
+++ test/FixIt/fixit-format-ios-nopedantic.m
@@ -0,0 +1,21 @@
+// RUN: cp %s %t
+// RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -fsyntax-only -Wformat -Werror -fixit %t
+
+int printf(const char *restrict, ...);
+typedef unsigned int NSUInteger;
+typedef int NSInteger;
+NSUInteger getNSUInteger();
+NSInteger getNSInteger();
+
+void test() {
+  // For thumbv7-apple-ios8.0.0 the underlying type of ssize_t is long
+  // and the underlying type of size_t is unsigned long.
+
+  printf("test 1: %zu", getNSUInteger());
+
+  printf("test 2: %zu %zu", getNSUInteger(), getNSUInteger());
+
+  printf("test 3: %zd", getNSInteger());
+
+  printf("test 4: %zd %zd", getNSInteger(), getNSInteger());
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -6610,11 +6610,11 @@
 ExprTy = TET->getUnderlyingExpr()->getType();
   }
 
-  analyze_printf::ArgType::MatchKind match = AT.matchesType(S.Context, ExprTy);
-
-  if (match == analyze_printf::ArgType::Match) {
+  const analyze_printf::ArgType::MatchKind Match =
+  AT.matchesType(S.Context, ExprTy);
+  bool Pedantic = Match == analyze_printf::ArgType::NoMatchPedantic;
+  if (Match == analyze_printf::ArgType::Match)
 return true;
-  }
 
   // Look through argument promotions for our error message's reported type.
   // This includes the integral and floating promotions, but excludes array
@@ -6690,32 +6690,37 @@
 QualType CastTy;
 std::tie(CastTy, CastTyName) = shouldNotPrintDirectly(S.Context, IntendedTy, E);
 if (!CastTy.isNull()) {
+  // %zi/%zu are OK to use for NSInteger/NSUInteger of type int
+  // (long in ASTContext). Only complain to pedants.
+  if ((CastTyName == "NSInteger" || CastTyName == "NSUInteger") &&
+  AT.isSizeT() && AT.matchesType(S.Context, CastTy))
+Pedantic = true;
   IntendedTy = CastTy;
   ShouldNotPrintDirectly = true;
 }
   }
 
   // We may be able to offer a FixItHint if it is a supported type.
   PrintfSpecifier fixedFS = FS;
-  bool success =
+  bool Success =
   fixedFS.fixType(IntendedTy, S.getLangOpts(), S.Context, isObjCContext());
 
-  if (success) {
+  if (Success) {
 // Get the fix string from the fixed format specifier
 SmallString<16> buf;
 llvm::raw_svector_ostream os(buf);
 fixedFS.toString(os);
 
 CharSourceRange SpecRange = getSpecifierRange(StartSpecifier, SpecifierLen);
 
 if (IntendedTy == ExprTy && !ShouldNotPrintDirectly) {
-  unsigned diag = diag::warn_format_conversion_argument_type_mismatch;
-  if (match == analyze_format_string::ArgType::NoMatchPedantic) {
-diag = diag::warn_format_conversion_argument_type_mismatch_

Re: [libcxx] r333325 - Add nonnull; use it for atomics

[Marshall Clow via cfe-commits]

On Fri, May 25, 2018 at 4:43 PM, JF Bastien via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: jfb
> Date: Fri May 25 16:43:53 2018
> New Revision: 25
>
> URL: http://llvm.org/viewvc/llvm-project?rev=25&view=rev
> Log:
> Add nonnull; use it for atomics
>
>
JF - please revert this patch.
Let's have a discussion about how to implement this so that it is more
friendly to people with installed code bases.
[ We've had *extremely* loud responses to unilaterally adding warnings -
especially ones that can't be easily disabled - to the libc++ code base in
the past. ]

Also, please include both myself and EricWF on all libc++ reviews.

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

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

Addressed comments.


Repository:
  rC Clang

https://reviews.llvm.org/D47290



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

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D47290#1113412, @jfb wrote:

> In https://reviews.llvm.org/D47290#1113365, @aaron.ballman wrote:
>
> > This is relaxing `-Wformat` and making users instead write 
> > `-Wformat-pedantic` to get the strong guarantees, which is the opposite of 
> > what I thought the consensus was. Have I misunderstood something?
>
>
> From Shoaib's email:
>
> > Based on all the discussion so far, I think the consensus is that we don't 
> > want to relax -Wformat by default, but an additional relaxed option would 
> > be fine. That works for me (where I can just switch my codebases to use the 
> > new warning option), but it wouldn't handle JF Bastien's use case, so he 
> > would still want to pursue the relaxations specific to Apple platforms.
>
> This patch is specific to the Darwin platform, as proposed in Alex' original 
> patch and as I promised to do in the cfe-dev thread. Shoaib's follow-up would 
> be different and do what I think you're referring to.


There were several people on the thread asking to not relax -Wformat for any 
target, but instead wanted a separate warning flag to perform a more relaxed 
check.

http://lists.llvm.org/pipermail/cfe-dev/2018-May/057938.html
http://lists.llvm.org/pipermail/cfe-dev/2018-May/058030.html
http://lists.llvm.org/pipermail/cfe-dev/2018-May/058039.html


Repository:
  rC Clang

https://reviews.llvm.org/D47290



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

r333340 - [ClangDiagnostics] Silence warning about fallthrough after PrintFatalError

[David Bolvansky via cfe-commits]

Author: xbolva00
Date: Sat May 26 02:24:00 2018
New Revision: 40

URL: http://llvm.org/viewvc/llvm-project?rev=40&view=rev
Log:
[ClangDiagnostics] Silence warning about fallthrough after PrintFatalError

Summary:
ClangDiagnosticsEmitter.cpp:1047:57: warning: this statement may fall through 
[-Wimplicit-fallthrough=]
   Builder.PrintFatalError("Unknown modifier type: " + Modifier);
   ~~^~
ClangDiagnosticsEmitter.cpp:1048:5: note: here
 case MT_Select: {
   ^

Reviewers: rsmith, rtrieu

Reviewed By: rtrieu

Subscribers: rtrieu, ilya-biryukov, ioeric, MaskRay, jkorous, cfe-commits

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

Modified:
cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp

Modified: cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp?rev=40&r1=39&r2=40&view=diff
==
--- cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp (original)
+++ cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp Sat May 26 02:24:00 
2018
@@ -633,7 +633,7 @@ struct DiagnosticTextBuilder {
 return It->second.Root;
   }
 
-  void PrintFatalError(llvm::Twine const &Msg) const {
+  LLVM_ATTRIBUTE_NORETURN void PrintFatalError(llvm::Twine const &Msg) const {
 assert(EvaluatingRecord && "not evaluating a record?");
 llvm::PrintFatalError(EvaluatingRecord->getLoc(), Msg);
   }


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

[libcxx] r333351 - Revert "Add nonnull; use it for atomics"

[JF Bastien via cfe-commits]

Author: jfb
Date: Sat May 26 12:44:45 2018
New Revision: 51

URL: http://llvm.org/viewvc/llvm-project?rev=51&view=rev
Log:
Revert "Add nonnull; use it for atomics"

That's r25, as well as follow-up "Fix GCC handling of ATOMIC_VAR_INIT"
r27.

Marshall asked to revert:

Let's have a discussion about how to implement this so that it is more friendly
to people with installed code bases. We've had *extremely* loud responses to
unilaterally adding warnings - especially ones that can't be easily disabled -
to the libc++ code base in the past.

Removed:
libcxx/trunk/test/libcxx/atomics/diagnose_nonnull.fail.cpp
Modified:
libcxx/trunk/include/__config
libcxx/trunk/include/atomic

Modified: libcxx/trunk/include/__config
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__config?rev=51&r1=50&r2=51&view=diff
==
--- libcxx/trunk/include/__config (original)
+++ libcxx/trunk/include/__config Sat May 26 12:44:45 2018
@@ -1218,17 +1218,6 @@ _LIBCPP_FUNC_VIS extern "C" void __sanit
 #  endif
 #endif
 
-#if __has_attribute(nonnull) || _GNUC_VER >= 400
-// Function pointer parameter must be non-null: warns on null parameter,
-// undefined behavior if the parameter is null. Omitting parameter indices
-// indicates that all parameters of pointer type cannot be null.
-//
-// Note: parameter indexing starts at 1.
-#  define _LIBCPP_NONNULL(...) __attribute__((nonnull(__VA_ARGS__)))
-#else
-#  define _LIBCPP_NONNULL(...)
-#endif
-
 // Define availability macros.
 #if defined(_LIBCPP_USE_AVAILABILITY_APPLE)
 #  define _LIBCPP_AVAILABILITY_SHARED_MUTEX
\

Modified: libcxx/trunk/include/atomic
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/atomic?rev=51&r1=50&r2=51&view=diff
==
--- libcxx/trunk/include/atomic (original)
+++ libcxx/trunk/include/atomic Sat May 26 12:44:45 2018
@@ -646,23 +646,19 @@ static inline _LIBCPP_CONSTEXPR int __to
 } // namespace __gcc_atomic
 
 template 
-_LIBCPP_NONNULL(1)
 static inline
 typename enable_if<
 __gcc_atomic::__can_assign::value>::type
-__c11_atomic_init(volatile _Atomic(_Tp)* __a,  _Tp __val)
-{
+__c11_atomic_init(volatile _Atomic(_Tp)* __a,  _Tp __val) {
   __a->__a_value = __val;
 }
 
 template 
-_LIBCPP_NONNULL(1)
 static inline
 typename enable_if<
 !__gcc_atomic::__can_assign::value &&
  __gcc_atomic::__can_assign< _Atomic(_Tp)*, _Tp>::value>::type
-__c11_atomic_init(volatile _Atomic(_Tp)* __a,  _Tp __val)
-{
+__c11_atomic_init(volatile _Atomic(_Tp)* __a,  _Tp __val) {
   // [atomics.types.generic]p1 guarantees _Tp is trivially copyable. Because
   // the default operator= in an object is not volatile, a byte-by-byte copy
   // is required.
@@ -675,9 +671,7 @@ __c11_atomic_init(volatile _Atomic(_Tp)*
 }
 
 template 
-_LIBCPP_NONNULL(1)
-static inline void __c11_atomic_init(_Atomic(_Tp)* __a,  _Tp __val)
-{
+static inline void __c11_atomic_init(_Atomic(_Tp)* __a,  _Tp __val) {
   __a->__a_value = __val;
 }
 
@@ -690,28 +684,22 @@ static inline void __c11_atomic_signal_f
 }
 
 template 
-_LIBCPP_NONNULL(1)
 static inline void __c11_atomic_store(volatile _Atomic(_Tp)* __a,  _Tp __val,
-  memory_order __order)
-{
+  memory_order __order) {
   return __atomic_store(&__a->__a_value, &__val,
 __gcc_atomic::__to_gcc_order(__order));
 }
 
 template 
-_LIBCPP_NONNULL(1)
 static inline void __c11_atomic_store(_Atomic(_Tp)* __a,  _Tp __val,
-  memory_order __order)
-{
+  memory_order __order) {
   __atomic_store(&__a->__a_value, &__val,
  __gcc_atomic::__to_gcc_order(__order));
 }
 
 template 
-_LIBCPP_NONNULL(1)
 static inline _Tp __c11_atomic_load(volatile _Atomic(_Tp)* __a,
-memory_order __order)
-{
+memory_order __order) {
   _Tp __ret;
   __atomic_load(&__a->__a_value, &__ret,
 __gcc_atomic::__to_gcc_order(__order));
@@ -719,9 +707,7 @@ static inline _Tp __c11_atomic_load(vola
 }
 
 template 
-_LIBCPP_NONNULL(1)
-static inline _Tp __c11_atomic_load(_Atomic(_Tp)* __a, memory_order __order)
-{
+static inline _Tp __c11_atomic_load(_Atomic(_Tp)* __a, memory_order __order) {
   _Tp __ret;
   __atomic_load(&__a->__a_value, &__ret,
 __gcc_atomic::__to_gcc_order(__order));
@@ -729,10 +715,8 @@ static inline _Tp __c11_atomic_load(_Ato
 }
 
 template 
-_LIBCPP_NONNULL(1)
 static inline _Tp __c11_atomic_exchange(volatile _Atomic(_Tp)* __a,
-_Tp __value, memory_order __order)
-{
+_Tp __value, memory_order __order) {
   _Tp __ret;
   __atomic_exchange(&__a->__a_va

Re: [libcxx] r333325 - Add nonnull; use it for atomics

[JF Bastien via cfe-commits]

> On May 26, 2018, at 12:36 PM, Marshall Clow  wrote:
> 
> 
> 
> On Fri, May 25, 2018 at 4:43 PM, JF Bastien via cfe-commits 
> mailto:cfe-commits@lists.llvm.org>> wrote:
> Author: jfb
> Date: Fri May 25 16:43:53 2018
> New Revision: 25
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=25&view=rev 
> 
> Log:
> Add nonnull; use it for atomics
> 
> 
> JF - please revert this patch.

r51

> Let's have a discussion about how to implement this so that it is more 
> friendly to people with installed code bases.
> [ We've had *extremely* loud responses to unilaterally adding warnings - 
> especially ones that can't be easily disabled - to the libc++ code base in 
> the past. ] 

Ha! Interesting point which I hadn’t considered. I guess the clang warning can 
be controlled, but this one cannot? Agreed that’s suboptimal and we should 
figure out a way to make them controllable.


> Also, please include both myself and EricWF on all libc++ reviews.

Sorry, will do. I assumed you’d all have Herald auto-add if you wanted to be on 
a patch :-)


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

[PATCH] D45686: [Driver] Clean up tmp files when deleting Compilation objects

[Karl-Johan Karlsson via Phabricator via cfe-commits]

Ka-Ka added a comment.

In https://reviews.llvm.org/D45686#1113414, @aaron.ballman wrote:

> In https://reviews.llvm.org/D45686#1109295, @dstenb wrote:
>
> > Ping.
> >
> > We have added a lit reproducer for this now in clang-tools-extra: 
> > https://reviews.llvm.org/D47251.
>
>
> The above should be rolled into this patch as the test case verifying the 
> behavioral changes.


I guess the intention from @dstenb was to submit both patches together. The 
reason for a separate patch with the testcase alone is that the reproducer use 
clang-tidy and as far as I know all clang-tidy tests exist in clang-tools-extra.


https://reviews.llvm.org/D45686



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

[PATCH] D45686: [Driver] Clean up tmp files when deleting Compilation objects

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D45686#1113425, @Ka-Ka wrote:

> In https://reviews.llvm.org/D45686#1113414, @aaron.ballman wrote:
>
> > In https://reviews.llvm.org/D45686#1109295, @dstenb wrote:
> >
> > > Ping.
> > >
> > > We have added a lit reproducer for this now in clang-tools-extra: 
> > > https://reviews.llvm.org/D47251.
> >
> >
> > The above should be rolled into this patch as the test case verifying the 
> > behavioral changes.
>
>
> I guess the intention from @dstenb was to submit both patches together. The 
> reason for a separate patch with the testcase alone is that the reproducer 
> use clang-tidy and as far as I know all clang-tidy tests exist in 
> clang-tools-extra.


.. and both of them exist in the **same** svn repo, or git monorepo. (granted, 
not many use that)


https://reviews.llvm.org/D45686



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

[PATCH] D45686: [Driver] Clean up tmp files when deleting Compilation objects

[Karl-Johan Karlsson via Phabricator via cfe-commits]

Ka-Ka added a comment.

In https://reviews.llvm.org/D45686#1113426, @lebedev.ri wrote:

> In https://reviews.llvm.org/D45686#1113425, @Ka-Ka wrote:
>
> > In https://reviews.llvm.org/D45686#1113414, @aaron.ballman wrote:
> >
> > > In https://reviews.llvm.org/D45686#1109295, @dstenb wrote:
> > >
> > > > Ping.
> > > >
> > > > We have added a lit reproducer for this now in clang-tools-extra: 
> > > > https://reviews.llvm.org/D47251.
> > >
> > >
> > > The above should be rolled into this patch as the test case verifying the 
> > > behavioral changes.
> >
> >
> > I guess the intention from @dstenb was to submit both patches together. The 
> > reason for a separate patch with the testcase alone is that the reproducer 
> > use clang-tidy and as far as I know all clang-tidy tests exist in 
> > clang-tools-extra.
>
>
> .. and both of them exist in the **same** svn repo, or git monorepo. 
> (granted, not many use that)


I see, I didn't know that. Well, then it make sense to merge both patches into 
a single one. Thanks for clarifying that.


https://reviews.llvm.org/D45686



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

[PATCH] D47135: [analyzer] A checker for dangling internal buffer pointers in C++

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:33
+public:
+  DanglingInternalBufferChecker() : CStrFn("c_str") {}
+

string.data() support?


https://reviews.llvm.org/D47135



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

[PATCH] D47401: [X86] Rewrite the max and min reduction intrinsics to make better use of other functions and to reduce width to 256 and 128 bits were possible.

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added inline comments.



Comment at: cfe/trunk/lib/Headers/avx512fintrin.h:9855
+  __v8di __t6 = (__v8di)_mm512_##op(__t4, __t5); \
+  return __t6[0];
 

Would it be dumb to allow VLX capable CPUs to use 128/256 variants of the 
VPMAXUQ etc ? Or is it better to focus on improving SimplifyDemandedElts to 
handle this (and many other reduction cases that all tend to keep to the 
original vector width)?


Repository:
  rL LLVM

https://reviews.llvm.org/D47401



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

[PATCH] D45517: [analyzer] WIP: False positive refutation with Z3

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

FYI the fix for the 1-bit APSInt issue is in 
https://reviews.llvm.org/D35450#change-ifYnQ3IlVso


https://reviews.llvm.org/D45517



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

[PATCH] D45517: [analyzer] WIP: False positive refutation with Z3

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added inline comments.



Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:1261
+
+  for (ConstraintRangeTy::iterator I = CR.begin(), E = CR.end(); I != E; ++I) {
+SymbolRef Sym = I.getKey();

for (auto I : CR)?


https://reviews.llvm.org/D45517



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

[PATCH] D46700: [ThinLTO] Add testing of new summary index format to a couple CFI tests

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson updated this revision to Diff 148740.
tejohnson added a comment.

Update tests so they won't get bot failures (don't try to match path, module
hash).

Ping - this can go in now that https://reviews.llvm.org/D46699 is in.


Repository:
  rC Clang

https://reviews.llvm.org/D46700

Files:
  test/CodeGen/thinlto-distributed-cfi-devirt.ll
  test/CodeGen/thinlto-distributed-cfi.ll


Index: test/CodeGen/thinlto-distributed-cfi.ll
===
--- test/CodeGen/thinlto-distributed-cfi.ll
+++ test/CodeGen/thinlto-distributed-cfi.ll
@@ -20,6 +20,11 @@
 ; CHECK: blob data = '_ZTS1A'
 ; CHECK-LABEL: Index: test/CodeGen/thinlto-distributed-cfi.ll
===
--- test/CodeGen/thinlto-distributed-cfi.ll
+++ test/CodeGen/thinlto-distributed-cfi.ll
@@ -20,6 +20,11 @@
 ; CHECK: blob data = '_ZTS1A'
 ; CHECK-LABEL: ___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47401: [X86] Rewrite the max and min reduction intrinsics to make better use of other functions and to reduce width to 256 and 128 bits were possible.

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: cfe/trunk/lib/Headers/avx512fintrin.h:9855
+  __v8di __t6 = (__v8di)_mm512_##op(__t4, __t5); \
+  return __t6[0];
 

RKSimon wrote:
> Would it be dumb to allow VLX capable CPUs to use 128/256 variants of the 
> VPMAXUQ etc ? Or is it better to focus on improving SimplifyDemandedElts to 
> handle this (and many other reduction cases that all tend to keep to the 
> original vector width)?
I'm not sure how to do that from clang. Should we be using a reduction 
intrinsic and do custom lowering in the backend?


Repository:
  rL LLVM

https://reviews.llvm.org/D47401



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