[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 145155.
baloghadamsoftware added a comment.

Retrying...


https://reviews.llvm.org/D33537

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/ExceptionEscapeCheck.cpp
  clang-tidy/bugprone/ExceptionEscapeCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-exception-escape.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-exception-escape.cpp

Index: test/clang-tidy/bugprone-exception-escape.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-exception-escape.cpp
@@ -0,0 +1,256 @@
+// RUN: %check_clang_tidy %s bugprone-exception-escape %t -- -extra-arg=-std=c++11 -config="{CheckOptions: [{key: bugprone-exception-escape.IgnoredExceptions, value: 'ignored1,ignored2'}, {key: bugprone-exception-escape.EnabledFunctions, value: 'enabled1,enabled2,enabled3'}]}" --
+
+struct throwing_destructor {
+  ~throwing_destructor() {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function '~throwing_destructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_constructor {
+  throwing_move_constructor(throwing_move_constructor&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'throwing_move_constructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_assignment {
+  throwing_move_assignment& operator=(throwing_move_assignment&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: function 'operator=' throws
+throw 1;
+  }
+};
+
+void throwing_noexcept() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_noexcept' throws
+  throw 1;
+}
+
+void throwing_throw_nothing() throw() {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_throw_nothing' throws
+  throw 1;
+}
+
+void throw_and_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch' throws
+  try {
+throw 1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_some(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_catch_some' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_each(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_each' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(double &) {
+  }
+}
+
+void throw_and_catch_all(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_all' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(...) {
+  }
+}
+
+void throw_and_rethrow() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_rethrow' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw;
+  }
+}
+
+void throw_catch_throw() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_throw' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw 2;
+  }
+}
+
+void throw_catch_rethrow_the_rest(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_rethrow_the_rest' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(...) {
+throw;
+  }
+}
+
+class base {};
+class derived: public base {};
+
+void throw_derived_catch_base() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_derived_catch_base' throws
+  try {
+throw derived();
+  } catch(base &) {
+  }
+}
+
+void try_nested_try(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_try' throws
+  try {
+try {
+  if (n) throw 1;
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void bad_try_nested_try(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_try_nested_try' throws
+  try {
+if (n) throw 1;
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void try_nested_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_catch' throws
+  try {
+try {
+  throw 1;
+} catch(int &) {
+  throw 1.1;
+}
+  } catch(double &) {
+  }
+}
+
+void catch_nested_try() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1; 
+} catch(int &) {
+}
+  }
+}
+
+void bad_catch_nested_try() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void implicit_int_thrower() {
+  throw 1;
+}
+
+void explicit_int_thrower() throw(int);
+
+void indirect_implicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'indirect_implicit' throws
+  implicit_int_th

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

In https://reviews.llvm.org/D33537#1086571, @alexfh wrote:

> It looks like you've missed some comments or uploaded a wrong patch.


The latter. Now I retried to upload the correct patch.


https://reviews.llvm.org/D33537



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

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added a comment.

How about tests for functions with conditional noexcept?




Comment at: docs/clang-tidy/checks/bugprone-exception-escape.rst:8-9
+* Destructors
+* Copy constructors
+* Copy assignment operators
+* The ``main()`` functions

Are copy constructors and assignment operators not supposed to throw? Or did 
you mean *move* constructors?



Comment at: docs/clang-tidy/checks/bugprone-exception-escape.rst:28
+
+   Comma separated list containing function names which should not throw. An
+   example for using this parameter is the function ``WinMain()`` in the

`EnabledFunctions` but they *should not* throw?


https://reviews.llvm.org/D33537



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

[PATCH] D45774: [analyzer] cover more cases where a Loc can be bound to constants

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl updated this revision to Diff 145156.
r.stahl marked 3 inline comments as done.
r.stahl added a comment.

addressed the comments, thanks!

If it looks good, please commit for me.


https://reviews.llvm.org/D45774

Files:
  lib/StaticAnalyzer/Core/RegionStore.cpp
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  test/Analysis/globals.cpp

Index: test/Analysis/globals.cpp
===
--- test/Analysis/globals.cpp
+++ test/Analysis/globals.cpp
@@ -0,0 +1,111 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+
+
+static const unsigned long long scull = 0;
+void static_int()
+{
+*(int*)scull = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+const unsigned long long cull = 0;
+void const_int()
+{
+*(int*)cull = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+static int * const spc = 0;
+void static_ptr()
+{
+*spc = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+int * const pc = 0;
+void const_ptr()
+{
+*pc = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+const unsigned long long cull_nonnull = 4;
+void nonnull_int()
+{
+*(int*)(cull_nonnull - 4) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+int * const pc_nonnull = (int*)sizeof(int);
+void nonnull_ptr()
+{
+*(pc_nonnull - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+int * const constcast = const_cast((int*)sizeof(int));
+void cast1()
+{
+*(constcast - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+int * const recast = reinterpret_cast(sizeof(int));
+void cast2()
+{
+*(recast - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+int * const staticcast = static_cast((int*)sizeof(int));
+void cast3()
+{
+*(staticcast - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+struct Foo { int a; };
+Foo * const dyncast = dynamic_cast((Foo*)sizeof(Foo));
+void cast4()
+{
+// Do not handle dynamic_cast for now, because it may change the pointer value.
+(dyncast - 1)->a = 0; // no-warning
+}
+
+typedef int * const intptrconst;
+int * const funccast = intptrconst(sizeof(int));
+void cast5()
+{
+*(funccast - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+struct S1
+{
+int * p;
+};
+const S1 s1 = {
+.p = (int*)sizeof(int)
+};
+void conststruct()
+{
+*(s1.p - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+struct S2
+{
+int * const p;
+};
+S2 s2 = {
+.p = (int*)sizeof(int)
+};
+void constfield()
+{
+*(s2.p - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+int * const parr[1] = { (int*)sizeof(int) };
+void constarr()
+{
+*(parr[0] - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+struct S3
+{
+int * p = (int*)sizeof(int);
+};
+void recordinit()
+{
+S3 s3;
+*(s3.p - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1606,7 +1606,7 @@
   const MemRegion* superR = R->getSuperRegion();
 
   // Check if the region is an element region of a string literal.
-  if (const StringRegion *StrR=dyn_cast(superR)) {
+  if (const StringRegion *StrR = dyn_cast(superR)) {
 // FIXME: Handle loads from strings where the literal is treated as
 // an integer, e.g., *((unsigned int*)"hello")
 QualType T = Ctx.getAsArrayType(StrR->getValueType())->getElementType();
@@ -1629,6 +1629,27 @@
   char c = (i >= length) ? '\0' : Str->getCodeUnit(i);
   return svalBuilder.makeIntVal(c, T);
 }
+  } else if (const VarRegion *VR = dyn_cast(superR)) {
+// Check if the containing array is const and has an initialized value.
+const VarDecl *VD = VR->getDecl();
+// Either the array or the array element has to be const.
+if (VD->getType().isConstQualified() || R->getElementType().isConstQualified()) {
+  if (const Expr *Init = VD->getInit()) {
+if (const auto *InitList = dyn_cast(Init)) {
+  // The array index has to be known.
+  if (auto CI = R->getIndex().getAs()) {
+int64_t i = CI->getValue().getSExtValue();
+// Return unknown value if index is out of bounds.
+if (i < 0 || i >= InitList->getNumInits())
+  return UnknownVal();
+
+if (const Expr *ElemInit = InitList->getInit(i))
+  if (Optional V = svalBuilder.getConstantVal(ElemInit))
+return *V;
+  }
+}
+  }
+}
   }
 
   // Check for loads from a code text region.  For such loads, just give up.
@@ -1678,7 +1699,28 @@
   if (const Optional &V = B.getDirectBinding(R))
 return *V;
 
-  QualType Ty = R->getValueType();
+  // Is the field declared constant and has an in-class initializer?
+  const FieldDecl *FD = R->getDec

[PATCH] D45774: [analyzer] cover more cases where a Loc can be bound to constants

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added inline comments.



Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1723
+if (const InitListExpr *InitList = dyn_cast(Init)) {
+  if (const Expr *FieldInit = InitList->getInit(FD->getFieldIndex())) {
+if (Optional V = svalBuilder.getConstantVal(FieldInit))

NoQ wrote:
> This method crashes when the index is out of range. Not sure - is it possible 
> to skip a few fields in the list? Would the list's AST automatically contain 
> any placeholders in this case?
Good catch, but I don't think this is an issue. When testing InitListExpr with 
missing inits, they get filled with ImplicitValueInitExpr.


https://reviews.llvm.org/D45774



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

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

[Mailing List "cfe-commits" via Phabricator via cfe-commits]

cfe-commits added a subscriber: junbuml.
cfe-commits added a comment.

I think you and Richard agreed that you weren’t going to synthesize a whole
expression tree at every use of the operator, and I agree with that
decision. That’s very different from what I’m asking you to do, which is to
synthesize in isolation a call to the copy-constructor. There are several
places in the compiler that require these implicit copies which aren’t just
normal expressions; this is the common pattern for handling them. The
synthesized expression can be emitted multiple times, and it can be freely
re-synthesized in different translation units instead of being serialized.
You’re already finding and caching a constructor; storing a
CXXConstructExpr is basically thr same thing, but in a nicer form that
handles more cases and doesn’t require as much redundant code in IRGen.

STLs *frequently* make use of default arguments on copy constructors (for
allocators). I agree that it’s unlikely that that would happen here, but
that’s precisely because it’s unlikely that this type would ever be
non-trivial.

Mostly, though, I don’t understand the point of imposing a partial set of
non-conformant restrictions on the type. It’s easy to support an arbitrary
copy constructor by synthesizing a CXXConstructExpr, and this will
magically take care of any constexpr issues, as well as removing the need
for open-coding a constructor call.

The constexpr issues are that certain references to constexpr variables of
literal type (as these types are likely to be) are required to not ODR-use
the variable but instead just directly produce the initializer as the
expression result.  That’s especially important here because (1) existing
STL binaries will not define these variables, and we shouldn’t create
artificial deployment problems for this feature, and (2) we’d really rather
not emit these expressions as loads from externally-defined variables that
the optimizer won’t be able to optimize.

John.

Eric Fiselier via Phabricator  wrote:

> EricWF added inline comments.
> 
> 
>  Comment at: lib/Sema/SemaDeclCXX.cpp:8931
>  +  /*ConstArg*/ true, false, false, false, false);
>  +  auto *CopyCtor = cast_or_null(SMI.getMethod());
>  +
> 
>  
> 
> rjmccall wrote:
>  > Sorry, I didn't realize you'd go off and write this code manually.
>  >
>  > The way we generally handle this sort of thing is just to synthesize an
>  expression in Sema that performs the copy-construction.  That way, the
>  stdlib can be as crazy as it wants — default arguments on the
>  copy-constructor or whatever else — and it just works.  The pattern to
>  follow here is the code in Sema::BuildExceptionDeclaration, except that in
>  your case you can completely dispense with faking up an OpaqueValueExpr and
>  instead just build a DeclRefExpr to the actual variable.  (You could even
>  use ActOnIdExpression for this, although just synthesizing the DRE
>  shouldn't be too hard.)  Then the ComparisonCategoryInfo can just store
>  expressions for each of the three (four?) variables, and in IRGen you just
>  evaluate the appropriate one into the destination.
>  I think this goes against the direction Richard and I decided to go, which
>  was to not synthesize any expressions.
> 
> The problem is that the synthesized expressions cannot be stored in
>  `ComparisonCategoryInfo`, because the data it contains is not serialized.
>  So when we read the AST back, the `ComparisonCategoryInfo` will be empty.
>  And for obvious reasons we can't cache the data in Sema either.
>  Additionally, we probably don't want to waste space building and storing
>  synthesized expressions in each AST node which needs them.
> 
> Although the checking here leaves something to be desired, I suspect it's
>  more than enough to handle any conforming STL implementation.
> 
> https://reviews.llvm.org/D45476



- F6099282: msg-9191-352.txt 


https://reviews.llvm.org/D45476



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

Re: [PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

[John McCall via cfe-commits]

I think you and Richard agreed that you weren’t going to synthesize a whole
expression tree at every use of the operator, and I agree with that
decision. That’s very different from what I’m asking you to do, which is to
synthesize in isolation a call to the copy-constructor. There are several
places in the compiler that require these implicit copies which aren’t just
normal expressions; this is the common pattern for handling them. The
synthesized expression can be emitted multiple times, and it can be freely
re-synthesized in different translation units instead of being serialized.
You’re already finding and caching a constructor; storing a
CXXConstructExpr is basically thr same thing, but in a nicer form that
handles more cases and doesn’t require as much redundant code in IRGen.

STLs *frequently* make use of default arguments on copy constructors (for
allocators). I agree that it’s unlikely that that would happen here, but
that’s precisely because it’s unlikely that this type would ever be
non-trivial.

Mostly, though, I don’t understand the point of imposing a partial set of
non-conformant restrictions on the type. It’s easy to support an arbitrary
copy constructor by synthesizing a CXXConstructExpr, and this will
magically take care of any constexpr issues, as well as removing the need
for open-coding a constructor call.

The constexpr issues are that certain references to constexpr variables of
literal type (as these types are likely to be) are required to not ODR-use
the variable but instead just directly produce the initializer as the
expression result.  That’s especially important here because (1) existing
STL binaries will not define these variables, and we shouldn’t create
artificial deployment problems for this feature, and (2) we’d really rather
not emit these expressions as loads from externally-defined variables that
the optimizer won’t be able to optimize.

John.

Eric Fiselier via Phabricator  wrote:

> EricWF added inline comments.
>
>
> 
> Comment at: lib/Sema/SemaDeclCXX.cpp:8931
> +  /*ConstArg*/ true, false, false, false, false);
> +  auto *CopyCtor = cast_or_null(SMI.getMethod());
> +
> 
> rjmccall wrote:
> > Sorry, I didn't realize you'd go off and write this code manually.
> >
> > The way we generally handle this sort of thing is just to synthesize an
> expression in Sema that performs the copy-construction.  That way, the
> stdlib can be as crazy as it wants — default arguments on the
> copy-constructor or whatever else — and it just works.  The pattern to
> follow here is the code in Sema::BuildExceptionDeclaration, except that in
> your case you can completely dispense with faking up an OpaqueValueExpr and
> instead just build a DeclRefExpr to the actual variable.  (You could even
> use ActOnIdExpression for this, although just synthesizing the DRE
> shouldn't be too hard.)  Then the ComparisonCategoryInfo can just store
> expressions for each of the three (four?) variables, and in IRGen you just
> evaluate the appropriate one into the destination.
> I think this goes against the direction Richard and I decided to go, which
> was to not synthesize any expressions.
>
> The problem is that the synthesized expressions cannot be stored in
> `ComparisonCategoryInfo`, because the data it contains is not serialized.
> So when we read the AST back, the `ComparisonCategoryInfo` will be empty.
> And for obvious reasons we can't cache the data in Sema either.
> Additionally, we probably don't want to waste space building and storing
> synthesized expressions in each AST node which needs them.
>
> Although the checking here leaves something to be desired, I suspect it's
> more than enough to handle any conforming STL implementation.
>
>
>
>
> https://reviews.llvm.org/D45476
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38845: [ASTImporter] Support importing UnresolvedMemberExpr, DependentNameType, DependentScopeDeclRefExpr

[Peter Szecsi via Phabricator via cfe-commits]

szepet updated this revision to Diff 145159.
szepet added a comment.

Format changes added based on comments.


https://reviews.llvm.org/D38845

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1503,5 +1503,60 @@
 ParameterizedTests, ImportFunctions,
 ::testing::Values(ArgVector(), ArgVector{"-fdelayed-template-parsing"}),);
 
+const internal::VariadicDynCastAllOfMatcher
+dependentScopeDeclRefExpr;
+
+TEST(ImportExpr, DependentScopeDeclRefExpr) {
+  MatchVerifier Verifier;
+  testImport("template  struct S { static T foo; };"
+ "template  void declToImport() {"
+ "  (void) S::foo;"
+ "}"
+ "void instantiate() { declToImport(); }",
+ Lang_CXX11, "", Lang_CXX11, Verifier,
+ functionTemplateDecl(has(functionDecl(has(compoundStmt(
+ has(cStyleCastExpr(has(dependentScopeDeclRefExpr());
+
+  testImport("template  struct S {"
+ "template static void foo(){};"
+ "};"
+ "template  void declToImport() {"
+ "  S::template foo();"
+ "}"
+ "void instantiate() { declToImport(); }",
+ Lang_CXX11, "", Lang_CXX11, Verifier,
+ functionTemplateDecl(has(functionDecl(has(compoundStmt(
+ has(callExpr(has(dependentScopeDeclRefExpr());
+}
+
+const internal::VariadicDynCastAllOfMatcher
+dependentNameType;
+
+TEST(ImportExpr, DependentNameType) {
+  MatchVerifier Verifier;
+  testImport("template  struct declToImport {"
+ "  typedef typename T::type dependent_name;"
+ "};",
+ Lang_CXX11, "", Lang_CXX11, Verifier,
+ classTemplateDecl(has(
+ cxxRecordDecl(has(typedefDecl(has(dependentNameType(;
+}
+
+const internal::VariadicDynCastAllOfMatcher
+unresolvedMemberExpr;
+
+TEST(ImportExpr, UnresolvedMemberExpr) {
+  MatchVerifier Verifier;
+  testImport("struct S { template  void mem(); };"
+ "template  void declToImport() {"
+ "  S s;"
+ "  s.mem();"
+ "}"
+ "void instantiate() { declToImport(); }",
+ Lang_CXX11, "", Lang_CXX11, Verifier,
+ functionTemplateDecl(has(functionDecl(has(
+ compoundStmt(has(callExpr(has(unresolvedMemberExpr());
+}
+
 } // end namespace ast_matchers
 } // end namespace clang
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -121,7 +121,7 @@
 QualType VisitSubstTemplateTypeParmType(const SubstTemplateTypeParmType *T);
 QualType VisitTemplateSpecializationType(const TemplateSpecializationType *T);
 QualType VisitElaboratedType(const ElaboratedType *T);
-// FIXME: DependentNameType
+QualType VisitDependentNameType(const DependentNameType *T);
 QualType VisitPackExpansionType(const PackExpansionType *T);
 QualType VisitDependentTemplateSpecializationType(
 const DependentTemplateSpecializationType *T);
@@ -347,8 +347,10 @@
 Expr *VisitCXXConstructExpr(CXXConstructExpr *E);
 Expr *VisitCXXMemberCallExpr(CXXMemberCallExpr *E);
 Expr *VisitCXXDependentScopeMemberExpr(CXXDependentScopeMemberExpr *E);
+Expr *VisitDependentScopeDeclRefExpr(DependentScopeDeclRefExpr *E);
 Expr *VisitCXXUnresolvedConstructExpr(CXXUnresolvedConstructExpr *CE);
 Expr *VisitUnresolvedLookupExpr(UnresolvedLookupExpr *E);
+Expr *VisitUnresolvedMemberExpr(UnresolvedMemberExpr *E);
 Expr *VisitExprWithCleanups(ExprWithCleanups *EWC);
 Expr *VisitCXXThisExpr(CXXThisExpr *E);
 Expr *VisitCXXBoolLiteralExpr(CXXBoolLiteralExpr *E);
@@ -938,6 +940,25 @@
 T->getKeyword(), Qualifier, Name, ToPack);
 }
 
+QualType ASTNodeImporter::VisitDependentNameType(const DependentNameType *T) {
+  NestedNameSpecifier *NNS = Importer.Import(T->getQualifier());
+  if (!NNS && T->getQualifier())
+return QualType();
+
+  IdentifierInfo *Name = Importer.Import(T->getIdentifier());
+  if (!Name && T->getIdentifier())
+return QualType();
+
+  QualType Canon = (T == T->getCanonicalTypeInternal().getTypePtr())
+   ? QualType()
+   : Importer.Import(T->getCanonicalTypeInternal());
+  if (!Canon.isNull())
+Canon = Canon.getCanonicalType();
+
+  return Importer.getToContext().getDependentNameType(T->getKeyword(), NNS,
+  Name, Canon);
+}
+
 QualType ASTNodeImporter::VisitObjCInterfaceType(const ObjCInterfaceType *T) {
   auto *Class =
   dyn_cast_or_null(Importer.Import(T->getDecl()));
@@ -6159,6 +6180,29 @@
   cast_or_null(ToFQ), MemberNameInfo, ResInfo);
 }
 
+Ex

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl created this revision.
r.stahl added reviewers: NoQ, dcoughlin, xazax.hun, george.karpenkov.
Herald added subscribers: cfe-commits, a.sidorin, rnkovacs, szepet.

The existing CTU mechanism imports FunctionDecls where the definition is 
available in another TU. This patch extends that to VarDecls, to bind more 
constants.

- Add VarDecl importing functionality to CrossTranslationUnitContext
- Import Decls while traversing them in AnalysisConsumer
- Add VarDecls to CTU external mappings generator


Repository:
  rC Clang

https://reviews.llvm.org/D46421

Files:
  include/clang/CrossTU/CrossTranslationUnit.h
  lib/CrossTU/CrossTranslationUnit.cpp
  lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  tools/clang-func-mapping/ClangFnMapGen.cpp

Index: tools/clang-func-mapping/ClangFnMapGen.cpp
===
--- tools/clang-func-mapping/ClangFnMapGen.cpp
+++ tools/clang-func-mapping/ClangFnMapGen.cpp
@@ -54,6 +54,7 @@
 
 private:
   void handleDecl(const Decl *D);
+  void addIfInMain(const DeclaratorDecl *DD, SourceLocation defStart);
 
   ASTContext &Ctx;
   llvm::StringMap Index;
@@ -65,35 +66,42 @@
 return;
 
   if (const auto *FD = dyn_cast(D)) {
-if (FD->isThisDeclarationADefinition()) {
-  if (const Stmt *Body = FD->getBody()) {
-std::string LookupName = CrossTranslationUnitContext::getLookupName(FD);
-const SourceManager &SM = Ctx.getSourceManager();
-if (CurrentFileName.empty()) {
-  CurrentFileName =
-  SM.getFileEntryForID(SM.getMainFileID())->tryGetRealPathName();
-  if (CurrentFileName.empty())
-CurrentFileName = "invalid_file";
-}
-
-switch (FD->getLinkageInternal()) {
-case ExternalLinkage:
-case VisibleNoLinkage:
-case UniqueExternalLinkage:
-  if (SM.isInMainFile(Body->getLocStart()))
-Index[LookupName] = CurrentFileName;
-default:
-  break;
-}
-  }
-}
+if (FD->isThisDeclarationADefinition())
+  if (const Stmt *Body = FD->getBody())
+addIfInMain(FD, Body->getLocStart());
+  } else if (const auto *VD = dyn_cast(D)) {
+if (VD->hasInit())
+  if (const Expr *Init = VD->getInit())
+addIfInMain(VD, Init->getLocStart());
   }
 
   if (const auto *DC = dyn_cast(D))
 for (const Decl *D : DC->decls())
   handleDecl(D);
 }
 
+void MapFunctionNamesConsumer::addIfInMain(const DeclaratorDecl *DD,
+   SourceLocation defStart) {
+  std::string LookupName = CrossTranslationUnitContext::getLookupName(DD);
+  const SourceManager &SM = Ctx.getSourceManager();
+  if (CurrentFileName.empty()) {
+CurrentFileName =
+SM.getFileEntryForID(SM.getMainFileID())->tryGetRealPathName();
+if (CurrentFileName.empty())
+  CurrentFileName = "invalid_file";
+  }
+
+  switch (DD->getLinkageInternal()) {
+  case ExternalLinkage:
+  case VisibleNoLinkage:
+  case UniqueExternalLinkage:
+if (SM.isInMainFile(defStart))
+  Index[LookupName] = CurrentFileName;
+  default:
+break;
+  }
+}
+
 class MapFunctionNamesAction : public ASTFrontendAction {
 protected:
   std::unique_ptr CreateASTConsumer(CompilerInstance &CI,
Index: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -346,6 +346,27 @@
 return true;
   }
 
+  bool VisitVarDecl(VarDecl *VD) {
+if (!Opts->naiveCTUEnabled())
+  return true;
+
+if (!VD->hasExternalStorage() ||
+VD->getAnyInitializer() != nullptr)
+  return true;
+
+llvm::Expected CTUDeclOrError =
+  CTU.getCrossTUDefinition(VD, Opts->getCTUDir(), Opts->getCTUIndexName());
+
+if (!CTUDeclOrError) {
+  handleAllErrors(CTUDeclOrError.takeError(),
+  [&](const cross_tu::IndexError &IE) {
+CTU.emitCrossTUDiagnostics(IE);
+  });
+}
+
+return true;
+  }
+
   bool VisitFunctionDecl(FunctionDecl *FD) {
 IdentifierInfo *II = FD->getIdentifier();
 if (II && II->getName().startswith("__inline"))
Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -111,52 +111,65 @@
   return Result.str();
 }
 
+bool HasDefinition(const FunctionDecl *D, const FunctionDecl *&DefD) {
+  return D->hasBody(DefD);
+}
+bool HasDefinition(const VarDecl *D, const VarDecl *&DefD) {
+  return D->getAnyInitializer(DefD) != nullptr;
+}
+template  bool HasDefinition(const T *D) {
+  const T *Unused;
+  return HasDefinition(D, Unused);
+}
+
 CrossTranslationUnitContext::CrossTranslationUnitContext(CompilerInstance &CI)
 : CI(CI), Context(CI.getASTContext()) {}
 
 CrossTranslationUnitContext

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

Tests and doc fixes pending. First I'm interested in your thoughts to this 
change.

It allows to bind more symbolic values to constants if they have been defined 
and initialized in another TU:

use.c:

  extern int * const p;
  extern struct S * const s;

def.c:

  int * const p = 0;
  struct S * const s = { /* complex init */ };


Repository:
  rC Clang

https://reviews.llvm.org/D46421



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

[PATCH] D40937: [clang-tidy] Infinite loop checker

[Peter Szecsi via Phabricator via cfe-commits]

szepet updated this revision to Diff 145160.
szepet marked 2 inline comments as done.
szepet added a comment.

Changes based on comments.


https://reviews.llvm.org/D40937

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/InfiniteLoopCheck.cpp
  clang-tidy/bugprone/InfiniteLoopCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-infinite-loop.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-infinite-loop.cpp

Index: test/clang-tidy/bugprone-infinite-loop.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-infinite-loop.cpp
@@ -0,0 +1,121 @@
+// RUN: %check_clang_tidy %s bugprone-infinite-loop %t
+
+void simple_infinite_loop1() {
+  int i = 0;
+  int j = 0;
+  while (i < 10) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the condition variable (i) is not updated in the loop body [bugprone-infinite-loop]
+j++;
+  }
+
+  do {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the condition variable (i) is not updated in the loop body
+j++;
+  } while (i < 10);
+
+  for (i = 0; i < 10; ++j) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the condition variable (i) is not updated in the loop body
+  }
+}
+
+void simple_infinite_loop2() {
+  int i = 0;
+  int j = 0;
+  int Limit = 10;
+  while (i < Limit) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: none of the condition variables (i, Limit) are updated in the loop body [bugprone-infinite-loop]
+j++;
+  }
+
+  do {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: none of the condition variables (i, Limit) are updated in the loop body
+j++;
+  } while (i < Limit);
+
+  for (i = 0; i < Limit; ++j) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: none of the condition variables (i, Limit) are updated in the loop body
+  }
+}
+
+void simple_not_infinite() {
+  int i = 0;
+  int Limit = 100;
+  while (i < Limit) { // Not an error since 'Limit' is updated
+Limit--;
+  }
+  do {
+Limit--;
+  } while (i < Limit);
+
+  for (i = 0; i < Limit; Limit--) {
+  }
+}
+
+void escape_before1() {
+  int i = 0;
+  int Limit = 100;
+  int *p = &i;
+  while (i < Limit) { // Not an error, since p is alias of i.
+*++p;
+  }
+
+  do {
+*++p;
+  } while (i < Limit);
+
+  for (i = 0; i < Limit; *++p) {
+;
+  }
+}
+
+void escape_before2() {
+  int i = 0;
+  int Limit = 100;
+  int *p = &i;
+  while (i < Limit) { // We do not warn since the var 'i' is escaped but it is
+  // an actual error, since the pointer 'p' is increased.
+*(p++);
+  }
+}
+
+void escape_after() {
+  int i = 0;
+  int j = 0;
+  int Limit = 10;
+
+  while (i < Limit) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: none of the condition variables (i, Limit) are updated in the loop body
+  }
+  int *p = &i;
+}
+
+int glob;
+void glob_var(int &x) {
+  int i = 0, Limit = 100;
+  while (x < Limit) { // Not an error since 'x' can be an alias of glob.
+glob++;
+  }
+}
+
+void glob_var2() {
+  int i = 0, Limit = 100;
+  while (glob < Limit) { // Since 'glob' is declared out of the function we do not warn.
+i++;
+  }
+}
+
+struct X {
+  void memberExpr_test(int i) {
+while (i < m) { // False negative: No warning, since skipping the case where
+// a memberExpr can be found in the condition.
+  ;
+}
+  }
+
+  void memberExpr_test2(int i) {
+while (i < m) {
+  --m;
+}
+  }
+  int m;
+};
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -29,6 +29,7 @@
bugprone-forwarding-reference-overload
bugprone-inaccurate-erase
bugprone-incorrect-roundings
+   bugprone-infinite-loop
bugprone-integer-division
bugprone-lambda-function-name
bugprone-macro-parentheses
Index: docs/clang-tidy/checks/bugprone-infinite-loop.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/bugprone-infinite-loop.rst
@@ -0,0 +1,30 @@
+.. title:: clang-tidy - bugprone-infinite-loop
+
+bugprone-infinite-loop
+==
+
+Finds loops where none of the condition variables are updated in the body. This
+performs a very conservative check in order to avoid false positives and work
+only on integer types at the moment.
+
+Examples:
+
+.. code-block:: c++
+
+  void simple_infinite_loop() {
+int i = 0;
+int j = 0;
+int Limit = 10;
+while (i < Limit) { // Error, since none of the variables are updated.
+  j++;
+}
+  }
+
+  void escape_before() {
+int i = 0;
+int Limit = 100;
+int *p = &i;
+while (i < Limit) { // Not an error, since p is alias of i.
+  *++p;
+}
+  }
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -70,6 +70,12 @@

[PATCH] D46398: [ASTImporterTest] Fix potential use-after-free

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: unittests/AST/ASTImporterTest.cpp:211
 StringRef Code;
 StringRef FileName;
 std::unique_ptr Unit;

Can't we have the same problem with FileName?

Perhaps an other alternative would be to make the members real strings.
```
std::string Code;
std::string FileName;
```


Repository:
  rC Clang

https://reviews.llvm.org/D46398



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

[PATCH] D45679: [clang-tidy] Add ExprMutationAnalyzer, that analyzes whether an expression is mutated within a statement.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tidy/utils/ExprMutationAnalyzer.cpp:88
+
+const Stmt *ExprMutationAnalyzer::findDirectMutation(const Expr *Exp) {
+  // LHS of any assignment operators.

shuaiwang wrote:
> aaron.ballman wrote:
> > Should this also consider a DeclRefExpr to a volatile-qualified variable as 
> > a direct mutation?
> > 
> > What about using `Expr::HasSideEffect()`?
> Good catch about DeclRefExpr to volatile.
> 
> `HasSideEffects` means something different. Here find*Mutation means find a 
> Stmt **in ancestors** that mutates the given Expr. `HasSideEffects` IIUC 
> means whether anything is mutated by the Expr but doesn't care about what 
> exactly is mutated.
May I ask the exact semantics for `volatile`? I would add the test to my check, 
too.

It is possible to write `const volatile int m = 42;`, but if i understand 
correctly `m` could still change by hardware, or would this be UB?
If the `const` is missing, one must always assume external modification, right?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45679



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

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

In https://reviews.llvm.org/D45476#1087446, @cfe-commits wrote:

> I think you and Richard agreed that you weren’t going to synthesize a whole
>  expression tree at every use of the operator, and I agree with that
>  decision. That’s very different from what I’m asking you to do, which is to
>  synthesize in isolation a call to the copy-constructor.


Perhaps. My apologies. I'm still quite new to the Clang internals. I appreciate 
your patience.

> There are several places in the compiler that require these implicit copies 
> which aren’t just
>  normal expressions; this is the common pattern for handling them. The
>  synthesized expression can be emitted multiple times, and it can be freely
>  re-synthesized in different translation units instead of being serialized.

I'm not sure this is always the case. For example:

  // foo.h
  #include 
  
  struct Foo {
int x;
  };
  inline auto operator<=>(Foo const& LHS, Foo const& RHS) {
  
  }
  // foo.cpp
  #include  // imported via module.
  auto bar(Foo LHS, Foo RHS) {
return 
  }



> You’re already finding and caching a constructor; storing a
>  CXXConstructExpr is basically thr same thing, but in a nicer form that
>  handles more cases and doesn’t require as much redundant code in IRGen.

I'm not actually caching the copy constructor. And I disagree that storing a
`CXXConstructExpr` is essentially the same thing. I can lookup the 
`CXXConstructorDecl` without `Sema`,
but I can't build a `CXXConstructExpr` without it.

> STLs *frequently* make use of default arguments on copy constructors (for
>  allocators). I agree that it’s unlikely that that would happen here, but
>  that’s precisely because it’s unlikely that this type would ever be
>  non-trivial.
> 
> Mostly, though, I don’t understand the point of imposing a partial set of
>  non-conformant restrictions on the type. It’s easy to support an arbitrary
>  copy constructor by synthesizing a CXXConstructExpr, and this will
>  magically take care of any constexpr issues, as well as removing the need
>  for open-coding a constructor call.
> 
> The constexpr issues are that certain references to constexpr variables of
>  literal type (as these types are likely to be) are required to not ODR-use
>  the variable but instead just directly produce the initializer as the
>  expression result.  That’s especially important here because (1) existing
>  STL binaries will not define these variables, and we shouldn’t create
>  artificial deployment problems for this feature, and (2) we’d really rather
>  not emit these expressions as loads from externally-defined variables that
>  the optimizer won’t be able to optimize.
> 
> John.


https://reviews.llvm.org/D45476



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

Re: [PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

[Eric Fiselier via cfe-commits]

Woops. Submitted that last comment too early. Editing it on Phab.

On Fri, May 4, 2018 at 2:31 AM, Eric Fiselier via Phabricator <
revi...@reviews.llvm.org> wrote:

> EricWF added a comment.
>
> In https://reviews.llvm.org/D45476#1087446, @cfe-commits wrote:
>
> > I think you and Richard agreed that you weren’t going to synthesize a
> whole
> >  expression tree at every use of the operator, and I agree with that
> >  decision. That’s very different from what I’m asking you to do, which
> is to
> >  synthesize in isolation a call to the copy-constructor.
>
>
> Perhaps. My apologies. I'm still quite new to the Clang internals. I
> appreciate your patience.
>
> > There are several places in the compiler that require these implicit
> copies which aren’t just
> >  normal expressions; this is the common pattern for handling them. The
> >  synthesized expression can be emitted multiple times, and it can be
> freely
> >  re-synthesized in different translation units instead of being
> serialized.
>
> I'm not sure this is always the case. For example:
>
>   // foo.h
>   #include 
>
>   struct Foo {
> int x;
>   };
>   inline auto operator<=>(Foo const& LHS, Foo const& RHS) {
>
>   }
>   // foo.cpp
>   #include  // imported via module.
>   auto bar(Foo LHS, Foo RHS) {
> return
>   }
>
>
>
> > You’re already finding and caching a constructor; storing a
> >  CXXConstructExpr is basically thr same thing, but in a nicer form that
> >  handles more cases and doesn’t require as much redundant code in IRGen.
>
> I'm not actually caching the copy constructor. And I disagree that storing
> a
> `CXXConstructExpr` is essentially the same thing. I can lookup the
> `CXXConstructorDecl` without `Sema`,
> but I can't build a `CXXConstructExpr` without it.
>
> > STLs *frequently* make use of default arguments on copy constructors (for
> >  allocators). I agree that it’s unlikely that that would happen here, but
> >  that’s precisely because it’s unlikely that this type would ever be
> >  non-trivial.
> >
> > Mostly, though, I don’t understand the point of imposing a partial set of
> >  non-conformant restrictions on the type. It’s easy to support an
> arbitrary
> >  copy constructor by synthesizing a CXXConstructExpr, and this will
> >  magically take care of any constexpr issues, as well as removing the
> need
> >  for open-coding a constructor call.
> >
> > The constexpr issues are that certain references to constexpr variables
> of
> >  literal type (as these types are likely to be) are required to not
> ODR-use
> >  the variable but instead just directly produce the initializer as the
> >  expression result.  That’s especially important here because (1)
> existing
> >  STL binaries will not define these variables, and we shouldn’t create
> >  artificial deployment problems for this feature, and (2) we’d really
> rather
> >  not emit these expressions as loads from externally-defined variables
> that
> >  the optimizer won’t be able to optimize.
> >
> > John.
>
>
> https://reviews.llvm.org/D45476
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46386: Adding __atomic_fetch_min/max intrinsics to clang

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

Is this some sort of a vendor extension then? OpenCL 1.2 atomic builtins don't 
have ordering parameter.


Repository:
  rC Clang

https://reviews.llvm.org/D46386



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

[PATCH] D46398: [ASTImporterTest] Fix potential use-after-free

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin added subscribers: pcc, klimek.
a.sidorin added a comment.

Hi Gabor,

> Can't we have the same problem with FileName?

As I can see, no. FileName is copied into std::string while building 
compilation arguments.

> Perhaps an other alternative would be to make the members real strings.

That was the initial solution and you was correct with it. However, it is still 
a workaround. The ultimate solution is to fix `buildASTFromCodeWithArgs()` so 
it can copy the buffer into a persistent storage if it is not null-terminated:

  StringRef NullTerminated = Code.toNullTerminatedStringRef(CodeStorage);
  auto CodeBuffer =
  CodeStorage.empty()
  ? llvm::MemoryBuffer::getMemBuffer(NullTerminated)
  : llvm::MemoryBuffer::getMemBufferCopy(NullTerminated);
  InMemoryFileSystem->addFile(FileNameRef, 0, std::move(CodeBuffer));

Unfortunately, this patch has two problems. The first one is double copying of 
the input Twine, but it can be avoided with some additional code. The second 
one is copying of null-terminated StringRefs which, I guess, cannot be avoided 
at all because there is no legal way to check if StringRef is null-terminated 
or not. So, I'm summoning the initial authors of the code for the help.

@pcc, @klimek Could you propose a better idea?


Repository:
  rC Clang

https://reviews.llvm.org/D46398



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

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 145172.
baloghadamsoftware added a comment.

Typo fixed.


https://reviews.llvm.org/D33537

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/ExceptionEscapeCheck.cpp
  clang-tidy/bugprone/ExceptionEscapeCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-exception-escape.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-exception-escape.cpp

Index: test/clang-tidy/bugprone-exception-escape.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-exception-escape.cpp
@@ -0,0 +1,256 @@
+// RUN: %check_clang_tidy %s bugprone-exception-escape %t -- -extra-arg=-std=c++11 -config="{CheckOptions: [{key: bugprone-exception-escape.IgnoredExceptions, value: 'ignored1,ignored2'}, {key: bugprone-exception-escape.EnabledFunctions, value: 'enabled1,enabled2,enabled3'}]}" --
+
+struct throwing_destructor {
+  ~throwing_destructor() {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function '~throwing_destructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_constructor {
+  throwing_move_constructor(throwing_move_constructor&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'throwing_move_constructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_assignment {
+  throwing_move_assignment& operator=(throwing_move_assignment&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: function 'operator=' throws
+throw 1;
+  }
+};
+
+void throwing_noexcept() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_noexcept' throws
+  throw 1;
+}
+
+void throwing_throw_nothing() throw() {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_throw_nothing' throws
+  throw 1;
+}
+
+void throw_and_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch' throws
+  try {
+throw 1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_some(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_catch_some' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_each(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_each' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(double &) {
+  }
+}
+
+void throw_and_catch_all(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_all' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(...) {
+  }
+}
+
+void throw_and_rethrow() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_rethrow' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw;
+  }
+}
+
+void throw_catch_throw() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_throw' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw 2;
+  }
+}
+
+void throw_catch_rethrow_the_rest(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_rethrow_the_rest' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(...) {
+throw;
+  }
+}
+
+class base {};
+class derived: public base {};
+
+void throw_derived_catch_base() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_derived_catch_base' throws
+  try {
+throw derived();
+  } catch(base &) {
+  }
+}
+
+void try_nested_try(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_try' throws
+  try {
+try {
+  if (n) throw 1;
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void bad_try_nested_try(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_try_nested_try' throws
+  try {
+if (n) throw 1;
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void try_nested_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_catch' throws
+  try {
+try {
+  throw 1;
+} catch(int &) {
+  throw 1.1;
+}
+  } catch(double &) {
+  }
+}
+
+void catch_nested_try() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1;
+} catch(int &) {
+}
+  }
+}
+
+void bad_catch_nested_try() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void implicit_int_thrower() {
+  throw 1;
+}
+
+void explicit_int_thrower() throw(int);
+
+void indirect_implicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'indirect_implicit' throws
+  implicit_int_thr

[PATCH] D46406: [docs] Fix typos in the Clang User's Manual.

[Hans Wennborg via Phabricator via cfe-commits]

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

Thanks!


Repository:
  rC Clang

https://reviews.llvm.org/D46406



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

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

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



Comment at: docs/clang-tidy/checks/bugprone-exception-escape.rst:8-9
+* Destructors
+* Copy constructors
+* Copy assignment operators
+* The ``main()`` functions

dberris wrote:
> Are copy constructors and assignment operators not supposed to throw? Or did 
> you mean *move* constructors?
You are right. In the description below it is correct, but here there was a 
typo.



Comment at: docs/clang-tidy/checks/bugprone-exception-escape.rst:28
+
+   Comma separated list containing function names which should not throw. An
+   example for using this parameter is the function ``WinMain()`` in the

dberris wrote:
> `EnabledFunctions` but they *should not* throw?
Maybe we should come up with a better name for this option. I just took this 
from our company internal check.


https://reviews.llvm.org/D33537



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

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 145174.
baloghadamsoftware marked an inline comment as done.
baloghadamsoftware added a comment.

New test added.


https://reviews.llvm.org/D33537

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/ExceptionEscapeCheck.cpp
  clang-tidy/bugprone/ExceptionEscapeCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-exception-escape.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-exception-escape.cpp

Index: test/clang-tidy/bugprone-exception-escape.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-exception-escape.cpp
@@ -0,0 +1,263 @@
+// RUN: %check_clang_tidy %s bugprone-exception-escape %t -- -extra-arg=-std=c++11 -config="{CheckOptions: [{key: bugprone-exception-escape.IgnoredExceptions, value: 'ignored1,ignored2'}, {key: bugprone-exception-escape.EnabledFunctions, value: 'enabled1,enabled2,enabled3'}]}" --
+
+struct throwing_destructor {
+  ~throwing_destructor() {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function '~throwing_destructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_constructor {
+  throwing_move_constructor(throwing_move_constructor&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'throwing_move_constructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_assignment {
+  throwing_move_assignment& operator=(throwing_move_assignment&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: function 'operator=' throws
+throw 1;
+  }
+};
+
+void throwing_noexcept() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_noexcept' throws
+  throw 1;
+}
+
+void throwing_throw_nothing() throw() {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_throw_nothing' throws
+  throw 1;
+}
+
+void throw_and_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch' throws
+  try {
+throw 1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_some(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_catch_some' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_each(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_each' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(double &) {
+  }
+}
+
+void throw_and_catch_all(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_all' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(...) {
+  }
+}
+
+void throw_and_rethrow() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_rethrow' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw;
+  }
+}
+
+void throw_catch_throw() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_throw' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw 2;
+  }
+}
+
+void throw_catch_rethrow_the_rest(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_rethrow_the_rest' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(...) {
+throw;
+  }
+}
+
+class base {};
+class derived: public base {};
+
+void throw_derived_catch_base() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_derived_catch_base' throws
+  try {
+throw derived();
+  } catch(base &) {
+  }
+}
+
+void try_nested_try(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_try' throws
+  try {
+try {
+  if (n) throw 1;
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void bad_try_nested_try(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_try_nested_try' throws
+  try {
+if (n) throw 1;
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void try_nested_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_catch' throws
+  try {
+try {
+  throw 1;
+} catch(int &) {
+  throw 1.1;
+}
+  } catch(double &) {
+  }
+}
+
+void catch_nested_try() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1;
+} catch(int &) {
+}
+  }
+}
+
+void bad_catch_nested_try() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void implicit_int_thrower() {
+  throw 1;
+}
+
+void explicit_int_thrower() throw(int);
+
+void indirect_implicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning

[PATCH] D46386: Adding __atomic_fetch_min/max intrinsics to clang

[Elena Demikhovsky via Phabricator via cfe-commits]

delena added a comment.

In https://reviews.llvm.org/D46386#1087533, @Anastasia wrote:

> Is this some sort of a vendor extension then? OpenCL 1.2 atomic builtins 
> don't have ordering parameter.


OpenCL 1.2 atomic builtins have relaxed semantics. Always, it is not parameter, 
it is defined behavior. I want to translate them to atomicrmw instruction and 
use one of clang intrinsics for this.
I can't use _sync_fetch_*, due to the different semantics. The __atomic_* allow 
to specify semantics, but min/max is missing in this set.


Repository:
  rC Clang

https://reviews.llvm.org/D46386



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

[PATCH] D45931: [ASTMatchers] Don't garble the profiling output when multiple TU's are processed

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

In https://reviews.llvm.org/D45931#1087007, @lebedev.ri wrote:

> In https://reviews.llvm.org/D45931#1086665, @alexfh wrote:
>
> > In https://reviews.llvm.org/D45931#1084503, @lebedev.ri wrote:
> >
> > > In https://reviews.llvm.org/D45931#1083192, @lebedev.ri wrote:
> > >
> > > > Thank you for looking at this.
> > > >
> > > > In https://reviews.llvm.org/D45931#1083184, @alexfh wrote:
> > > >
> > > > > From a user's perspective I'd probably prefer a different behavior of 
> > > > > checks profiling with multiple translation units: per-file table 
> > > > > after each file and an aggregate table at the end.
> > > >
> > > >
> > > > Is this a review note, or a general observation?
> > >
> >
> >
> > Why not both? ;)
>
>
> BTW, that did not answer the question:
>
> > I'm sure it could be done, i'm just not too sure how useful it would be, 
> > since it seems no one before now even noticed that timing with multiple 
> > TU's was 'broken'.


I think, I noticed incorrect handling of profiling with multiple TUs at some 
point and switched to running clang-tidy multiple times. The solution I 
proposed (most importantly, point 1) would be the most convenient one for me at 
that time:

> I'd probably go with a set of features enough for various use cases:
> 
> 0. don't add any profile merging logic to clang-tidy
> 
> 1. dump profile after each TU to the screen in the current tabulated format
> 2. add a flag to specify a file name prefix to dump profile output for each 
> file as CSV
> 3. (optional) add a script to merge profiles from CSV files and dump as CSV 
> or tabulated (without a script this could be done in a spreadsheet)

Are any other questions still open?


Repository:
  rC Clang

https://reviews.llvm.org/D45931



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

[PATCH] D45093: [AST] Fix -ast-print for _Bool when have diagnostics

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

This approach generally looks good to me, but I'd like @rsmith's opinion on 
whether we should be trying to make -ast-print have good source fidelity or 
not. I was under the impression we wanted -ast-print to faithfully reproduce 
code at least as a low priority desire, but it sounds like it may only be 
intended as an approximation of the user's source code, so adding extra 
machinery to support better fidelity may be more maintenance burden than it's 
worth.


https://reviews.llvm.org/D45093



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

[PATCH] D45702: [clang-tidy] Add a new check, readability-redundant-data-call, that finds and removes redundant calls to .data().

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D45702#1086802, @shuaiwang wrote:

> In https://reviews.llvm.org/D45702#1086250, @aaron.ballman wrote:
>
> > In https://reviews.llvm.org/D45702#1085890, @shuaiwang wrote:
> >
> > > In https://reviews.llvm.org/D45702#1085224, @aaron.ballman wrote:
> > >
> > > > > Have you run this over any large code bases to see whether the check 
> > > > > triggers in practice?
> > > >
> > > > I'm still curious about this, btw.
> > >
> > >
> > > Yes it triggers in Google's code base.
> >
> >
> > Were there any false positives that you saw?
>
>
> From randomly checking several triggerings no I didn't find any false 
> positives. I feel the check should be pretty safe in terms of false positives 
> because we only trigger on configured types.


Good to hear, thank you for the information.




Comment at: clang-tidy/readability/RedundantDataCallCheck.cpp:45
+  anyOf(TypesMatcher, pointerType(pointee(TypesMatcher)),
+  callee(namedDecl(hasName("data"
+  .bind("call",

shuaiwang wrote:
> aaron.ballman wrote:
> > Eugene.Zelenko wrote:
> > > aaron.ballman wrote:
> > > > Should this check apply equally to `std::string::c_str()` as well as 
> > > > `std::string::data()`?
> > > readability-redundant-string-cstr do both.
> > Yup! But that makes me wonder if the name "redundant-data-call" is an 
> > issue. Perhaps the check name should focus more on the array subscript in 
> > the presence of an operator[]()?
> How about "readability-circumlocutionary-subscript"?
> "readability-circumlocutionary-element-access"?
> "circumlocutionary" -> "verbose"?
hah, I think circumlocutionary might be a bit too much. ;-) I think 
`readability-simplify-array-subscript-expr` might be reasonable, however. Right 
now, the simplification is just for `foo.data()[0]` but it seems plausible that 
there are other array subscript simplifications that could be added in the 
future, like `a[1 + 1]` being converted to `a[2]` or `x ? a[200] : a[400]` 
going to `a[x ? 200 : 400]` (etc).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45702



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

[PATCH] D45679: [clang-tidy] Add ExprMutationAnalyzer, that analyzes whether an expression is mutated within a statement.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/utils/ExprMutationAnalyzer.cpp:88
+
+const Stmt *ExprMutationAnalyzer::findDirectMutation(const Expr *Exp) {
+  // LHS of any assignment operators.

JonasToth wrote:
> shuaiwang wrote:
> > aaron.ballman wrote:
> > > Should this also consider a DeclRefExpr to a volatile-qualified variable 
> > > as a direct mutation?
> > > 
> > > What about using `Expr::HasSideEffect()`?
> > Good catch about DeclRefExpr to volatile.
> > 
> > `HasSideEffects` means something different. Here find*Mutation means find a 
> > Stmt **in ancestors** that mutates the given Expr. `HasSideEffects` IIUC 
> > means whether anything is mutated by the Expr but doesn't care about what 
> > exactly is mutated.
> May I ask the exact semantics for `volatile`? I would add the test to my 
> check, too.
> 
> It is possible to write `const volatile int m = 42;`, but if i understand 
> correctly `m` could still change by hardware, or would this be UB?
> If the `const` is missing, one must always assume external modification, 
> right?
> HasSideEffects means something different. Here find*Mutation means find a 
> Stmt in ancestors that mutates the given Expr. HasSideEffects IIUC means 
> whether anything is mutated by the Expr but doesn't care about what exactly 
> is mutated.

What I am getting at is that the code in `findDirectMutation()` seems to go 
through a lot of effort looking for expressions that have side effects and I 
was wondering if we could leverage that call instead of duplicating the effort.



Comment at: clang-tidy/utils/ExprMutationAnalyzer.cpp:88
+
+const Stmt *ExprMutationAnalyzer::findDirectMutation(const Expr *Exp) {
+  // LHS of any assignment operators.

aaron.ballman wrote:
> JonasToth wrote:
> > shuaiwang wrote:
> > > aaron.ballman wrote:
> > > > Should this also consider a DeclRefExpr to a volatile-qualified 
> > > > variable as a direct mutation?
> > > > 
> > > > What about using `Expr::HasSideEffect()`?
> > > Good catch about DeclRefExpr to volatile.
> > > 
> > > `HasSideEffects` means something different. Here find*Mutation means find 
> > > a Stmt **in ancestors** that mutates the given Expr. `HasSideEffects` 
> > > IIUC means whether anything is mutated by the Expr but doesn't care about 
> > > what exactly is mutated.
> > May I ask the exact semantics for `volatile`? I would add the test to my 
> > check, too.
> > 
> > It is possible to write `const volatile int m = 42;`, but if i understand 
> > correctly `m` could still change by hardware, or would this be UB?
> > If the `const` is missing, one must always assume external modification, 
> > right?
> > HasSideEffects means something different. Here find*Mutation means find a 
> > Stmt in ancestors that mutates the given Expr. HasSideEffects IIUC means 
> > whether anything is mutated by the Expr but doesn't care about what exactly 
> > is mutated.
> 
> What I am getting at is that the code in `findDirectMutation()` seems to go 
> through a lot of effort looking for expressions that have side effects and I 
> was wondering if we could leverage that call instead of duplicating the 
> effort.
> May I ask the exact semantics for volatile? I would add the test to my check, 
> too.

Basically, volatile objects can change in ways unknown to the implementation, 
so a volatile read must always perform an actual read (because the object's 
value may have changed since the last read) and a volatile write must always be 
performed as well (because writing a value may have further side effects if the 
object is backed by some piece of interesting hardware).

> It is possible to write const volatile int m = 42;, but if i understand 
> correctly m could still change by hardware, or would this be UB?

That could still be changed by hardware, and it would not be UB. For instance, 
the variable might be referencing some hardware sensor and so you can only read 
the values in (hence it's const) but the values may be changed out from under 
you (hence, it's not constant).

> If the const is missing, one must always assume external modification, right?

You have to assume external modification regardless.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45679



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

[PATCH] D45944: Disallow pointers to const in __sync_fetch_and_xxx

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: rjmccall, efriedma.
aaron.ballman added a comment.

Adding a few more potential reviewers.


https://reviews.llvm.org/D45944



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

[PATCH] D38845: [ASTImporter] Support importing UnresolvedMemberExpr, DependentNameType, DependentScopeDeclRefExpr

[Aleksei Sidorin via Phabricator via cfe-commits]

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

LGTM!


https://reviews.llvm.org/D38845



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

[PATCH] D45815: [libclang] Allow skipping function bodies in preamble only

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Sorry for the delay


Repository:
  rC Clang

https://reviews.llvm.org/D45815



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

[PATCH] D45815: [libclang] Allow skipping function bodies in preamble only

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: include/clang/Frontend/ASTUnit.h:104
 
+  enum class SkipFunctionBodiesScope { None, MainFileAndPreamble, Preamble };
+

Maybe move this out of `ASTUnit`? Would allow removing the first qualifier in 
usages outside the class itself (i.e. 
`ASTUnit::SkipFunctionBodiesScope::Preamble` will be shorter!)



Comment at: include/clang/Frontend/ASTUnit.h:370
+  IntrusiveRefCntPtr VFS,
+  SkipFunctionBodiesScope SkipFunctionBodiesScp =
+  SkipFunctionBodiesScope::None,

NIT: Maybe keep the name `SkipFunctionBodies`? It seems that putting `Scp` at 
the end does not add any clarity.



Comment at: include/clang/Frontend/ASTUnit.h:831
ArrayRef RemappedFiles = None,
+   SkipFunctionBodiesScope SkipFunctionBodiesScp =
+   SkipFunctionBodiesScope::None,

This parameter seems slightly out of place.
The scope of skipped function bodies should be a property of the whole 
`ASTUnit`, changing it on every reparse would mean more complexity and wouldn't 
be used anyway.

I suggest storing a field of type `SkipFunctionBodiesScope` instead of adding 
parameters to `Reparse` and `LoadFromCompilerInvocation`.


Repository:
  rC Clang

https://reviews.llvm.org/D45815



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

[PATCH] D46431: [x86] Introduce the pconfig intrinsic

[Gabor Buella via Phabricator via cfe-commits]

GBuella created this revision.
GBuella added reviewers: craig.topper, zvi.
Herald added subscribers: cfe-commits, mgorny.

Repository:
  rC Clang

https://reviews.llvm.org/D46431

Files:
  include/clang/Driver/Options.td
  lib/Basic/Targets/X86.cpp
  lib/Basic/Targets/X86.h
  lib/Headers/CMakeLists.txt
  lib/Headers/cpuid.h
  lib/Headers/module.modulemap
  lib/Headers/pconfigintrin.h
  lib/Headers/x86intrin.h
  test/Driver/x86-target-features.c
  test/Headers/pconfigintin.c
  test/Preprocessor/predefined-arch-macros.c

Index: test/Preprocessor/predefined-arch-macros.c
===
--- test/Preprocessor/predefined-arch-macros.c
+++ test/Preprocessor/predefined-arch-macros.c
@@ -1204,6 +1204,7 @@
 // CHECK_ICX_M32: #define __MMX__ 1
 // CHECK_ICX_M32: #define __MPX__ 1
 // CHECK_ICX_M32: #define __PCLMUL__ 1
+// CHECK_ICX_M32: #define __PCONFIG__ 1
 // CHECK_ICX_M32: #define __PKU__ 1
 // CHECK_ICX_M32: #define __POPCNT__ 1
 // CHECK_ICX_M32: #define __PRFCHW__ 1
@@ -1261,6 +1262,7 @@
 // CHECK_ICX_M64: #define __MMX__ 1
 // CHECK_ICX_M64: #define __MPX__ 1
 // CHECK_ICX_M64: #define __PCLMUL__ 1
+// CHECK_ICX_M64: #define __PCONFIG__ 1
 // CHECK_ICX_M64: #define __PKU__ 1
 // CHECK_ICX_M64: #define __POPCNT__ 1
 // CHECK_ICX_M64: #define __PRFCHW__ 1
Index: test/Headers/pconfigintin.c
===
--- /dev/null
+++ test/Headers/pconfigintin.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 %s -ffreestanding -triple x86_64-unknown-unknown -target-feature +pconfig -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-64
+// RUN: %clang_cc1 %s -ffreestanding -triple i386 -target-feature +pconfig -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-32
+
+#include 
+#include 
+#include 
+
+uint32_t test_pconfig(uint32_t leaf, size_t data[3]) {
+// CHECK-64: call { i32, i64, i64, i64 } asm "pconfig", "={ax},={bx},={cx},={dx},{ax},{bx},{cx},{dx},~{cc},~{dirflag},~{fpsr},~{flags}"(i32 %{{.*}}, i64 %{{.*}}, i64 %{{.*}}, i64 %{{.*}})
+// CHECK-32: call { i32, i32, i32, i32 } asm "pconfig", "={ax},={bx},={cx},={dx},{ax},{bx},{cx},{dx},~{cc},~{dirflag},~{fpsr},~{flags}"(i32 %{{.*}}, i32 %{{.*}}, i32 %{{.*}}, i32 %{{.*}})
+  return _pconfig_u32(leaf, data);
+}
Index: test/Driver/x86-target-features.c
===
--- test/Driver/x86-target-features.c
+++ test/Driver/x86-target-features.c
@@ -159,3 +159,8 @@
 // RUN: %clang -target i386-unknown-linux-gnu -march=i386 -mno-movdir64b %s -### -o %t.o 2>&1 | FileCheck -check-prefix=NO-MOVDIR64B %s
 // MOVDIR64B: "-target-feature" "+movdir64b"
 // NO-MOVDIR64B: "-target-feature" "-movdir64b"
+
+// RUN: %clang -target i386-unknown-linux-gnu -march=i386 -mpconfig %s -### -o %t.o 2>&1 | FileCheck -check-prefix=PCONFIG %s
+// RUN: %clang -target i386-unknown-linux-gnu -march=i386 -mno-pconfig %s -### -o %t.o 2>&1 | FileCheck -check-prefix=NO-PCONFIG %s
+// PCONFIG: "-target-feature" "+pconfig"
+// NO-PCONFIG: "-target-feature" "-pconfig"
Index: lib/Headers/x86intrin.h
===
--- lib/Headers/x86intrin.h
+++ lib/Headers/x86intrin.h
@@ -105,4 +105,8 @@
 #include 
 #endif
 
+#if !defined(_MSC_VER) || __has_feature(modules) || defined(__PCONFIG__)
+#include 
+#endif
+
 #endif /* __X86INTRIN_H */
Index: lib/Headers/pconfigintrin.h
===
--- /dev/null
+++ lib/Headers/pconfigintrin.h
@@ -0,0 +1,50 @@
+/*=== pconfigintrin.h - X86 platform configuration -===
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ *===---===
+ */
+
+#ifndef __X86INTRIN_H
+#error "Never use  directly; include  instead."
+#endif
+
+#ifndef __PCONFIGINTRIN_H
+#define __PCONFIGINTRIN_H
+
+#define __MKTME_KEY_PROGRAM 0x0001
+
+

[PATCH] D44387: [x86] Introduce the pconfig/encl[u|s|v] intrinsics

[Gabor Buella via Phabricator via cfe-commits]

GBuella added a comment.

Here is a variation on this, using inline asm:
https://reviews.llvm.org/D46431


https://reviews.llvm.org/D44387



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

[PATCH] D35110: [Analyzer] Constraint Manager Negates Difference

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 145187.
baloghadamsoftware added a comment.

Fixed according to the comments.


https://reviews.llvm.org/D35110

Files:
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  test/Analysis/constraint_manager_negate_difference.c
  test/Analysis/ptr-arith.c

Index: test/Analysis/ptr-arith.c
===
--- test/Analysis/ptr-arith.c
+++ test/Analysis/ptr-arith.c
@@ -265,49 +265,26 @@
   clang_analyzer_eval((rhs - lhs) > 0); // expected-warning{{TRUE}}
 }
 
-//---
-// False positives
-//---
-
 void zero_implies_reversed_equal(int *lhs, int *rhs) {
   clang_analyzer_eval((rhs - lhs) == 0); // expected-warning{{UNKNOWN}}
   if ((rhs - lhs) == 0) {
-#ifdef ANALYZER_CM_Z3
 clang_analyzer_eval(rhs != lhs); // expected-warning{{FALSE}}
 clang_analyzer_eval(rhs == lhs); // expected-warning{{TRUE}}
-#else
-clang_analyzer_eval(rhs != lhs); // expected-warning{{UNKNOWN}}
-clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
-#endif
 return;
   }
   clang_analyzer_eval((rhs - lhs) == 0); // expected-warning{{FALSE}}
-#ifdef ANALYZER_CM_Z3
   clang_analyzer_eval(rhs == lhs); // expected-warning{{FALSE}}
   clang_analyzer_eval(rhs != lhs); // expected-warning{{TRUE}}
-#else
-  clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
-  clang_analyzer_eval(rhs != lhs); // expected-warning{{UNKNOWN}}
-#endif
 }
 
 void canonical_equal(int *lhs, int *rhs) {
   clang_analyzer_eval(lhs == rhs); // expected-warning{{UNKNOWN}}
   if (lhs == rhs) {
-#ifdef ANALYZER_CM_Z3
 clang_analyzer_eval(rhs == lhs); // expected-warning{{TRUE}}
-#else
-clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
-#endif
 return;
   }
   clang_analyzer_eval(lhs == rhs); // expected-warning{{FALSE}}
-
-#ifdef ANALYZER_CM_Z3
   clang_analyzer_eval(rhs == lhs); // expected-warning{{FALSE}}
-#else
-  clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
-#endif
 }
 
 void compare_element_region_and_base(int *p) {
Index: test/Analysis/constraint_manager_negate_difference.c
===
--- /dev/null
+++ test/Analysis/constraint_manager_negate_difference.c
@@ -0,0 +1,66 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection,core.builtin -analyzer-config aggressive-relational-comparison-simplification=true -verify %s
+
+void clang_analyzer_eval(int);
+
+void exit(int);
+
+#define UINT_MAX (~0U)
+#define INT_MAX (UINT_MAX & (UINT_MAX >> 1))
+
+extern void __assert_fail (__const char *__assertion, __const char *__file,
+unsigned int __line, __const char *__function)
+ __attribute__ ((__noreturn__));
+#define assert(expr) \
+  ((expr)  ? (void)(0)  : __assert_fail (#expr, __FILE__, __LINE__, __func__))
+
+void assert_in_range(int x) {
+  assert(x <= ((int)INT_MAX / 4));
+  assert(x >= -(((int)INT_MAX) / 4));
+}
+
+void assert_in_range_2(int m, int n) {
+  assert_in_range(m);
+  assert_in_range(n);
+}
+
+void equal(int m, int n) {
+  assert_in_range_2(m, n);
+  if (m != n)
+return;
+  clang_analyzer_eval(n == m); // expected-warning{{TRUE}}
+}
+
+void non_equal(int m, int n) {
+  assert_in_range_2(m, n);
+  if (m == n)
+return;
+  clang_analyzer_eval(n != m); // expected-warning{{TRUE}}
+}
+
+void less_or_equal(int m, int n) {
+  assert_in_range_2(m, n);
+  if (m < n)
+return;
+  clang_analyzer_eval(n <= m); // expected-warning{{TRUE}}
+}
+
+void less(int m, int n) {
+  assert_in_range_2(m, n);
+  if (m <= n)
+return;
+  clang_analyzer_eval(n < m); // expected-warning{{TRUE}}
+}
+
+void greater_or_equal(int m, int n) {
+  assert_in_range_2(m, n);
+  if (m > n)
+return;
+  clang_analyzer_eval(n >= m); // expected-warning{{TRUE}}
+}
+
+void greater(int m, int n) {
+  assert_in_range_2(m, n);
+  if (m >= n)
+return;
+  clang_analyzer_eval(n > m); // expected-warning{{TRUE}}
+}
Index: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -256,6 +256,25 @@
 return newRanges;
   }
 
+  // Turn all [A, B] ranges to [-B, -A]. Turn minimal signed value to maximal
+  // signed value.
+  RangeSet Negate(BasicValueFactory &BV, Factory &F) const {
+PrimRangeSet newRanges = F.getEmptySet();
+
+for (iterator i = begin(), e = end(); i != e; ++i) {
+  const llvm::APSInt &from = i->From(), &to = i->To();
+  const llvm::APSInt &newFrom = (to.isMinSignedValue() ?
+ BV.getMaxValue(to) :
+ BV.getValue(- to));
+  const llvm::APSInt &newTo = (from.isMinSignedValue() ?
+   BV.getMaxValue(from) :
+   BV.getValue(- from));
+  

[PATCH] D35110: [Analyzer] Constraint Manager Negates Difference

[Balogh , Ádám via Phabricator via cfe-commits]

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

I chose option 1 for now.


https://reviews.llvm.org/D35110



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

r331519 - [Coroutines] Catch exceptions in await_resume

[Brian Gesiak via cfe-commits]

Author: modocache
Date: Fri May  4 07:02:37 2018
New Revision: 331519

URL: http://llvm.org/viewvc/llvm-project?rev=331519&view=rev
Log:
[Coroutines] Catch exceptions in await_resume

Summary:
http://wg21.link/P0664r2 section "Evolution/Core Issues 24" describes a
proposed change to Coroutines TS that would have any exceptions thrown
after the initial suspend point of a coroutine be caught by the handler
specified by the promise type's 'unhandled_exception' member function.
This commit provides a sample implementation of the specified behavior.

Test Plan: `check-clang`

Reviewers: GorNishanov, EricWF

Reviewed By: GorNishanov

Subscribers: cfe-commits, lewissbaker, eric_niebler

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

Added:
cfe/trunk/test/CodeGenCoroutines/coro-await-resume-eh.cpp
Modified:
cfe/trunk/lib/CodeGen/CGCoroutine.cpp
cfe/trunk/test/CodeGenCoroutines/coro-unhandled-exception.cpp

Modified: cfe/trunk/lib/CodeGen/CGCoroutine.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCoroutine.cpp?rev=331519&r1=331518&r2=331519&view=diff
==
--- cfe/trunk/lib/CodeGen/CGCoroutine.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCoroutine.cpp Fri May  4 07:02:37 2018
@@ -44,6 +44,15 @@ struct clang::CodeGen::CGCoroData {
   // A branch to this block is emitted when coroutine needs to suspend.
   llvm::BasicBlock *SuspendBB = nullptr;
 
+  // The promise type's 'unhandled_exception' handler, if it defines one.
+  Stmt *ExceptionHandler = nullptr;
+
+  // A temporary i1 alloca that stores whether 'await_resume' threw an
+  // exception. If it did, 'true' is stored in this variable, and the coroutine
+  // body must be skipped. If the promise type does not define an exception
+  // handler, this is null.
+  llvm::Value *ResumeEHVar = nullptr;
+
   // Stores the jump destination just before the coroutine memory is freed.
   // This is the destination that every suspend point jumps to for the cleanup
   // branch.
@@ -208,11 +217,32 @@ static LValueOrRValue emitSuspendExpress
 
   // Emit await_resume expression.
   CGF.EmitBlock(ReadyBlock);
+  CXXTryStmt *TryStmt = nullptr;
+  if (Coro.ExceptionHandler && Kind == AwaitKind::Init) {
+Coro.ResumeEHVar =
+CGF.CreateTempAlloca(Builder.getInt1Ty(), Prefix + Twine("resume.eh"));
+Builder.CreateFlagStore(true, Coro.ResumeEHVar);
+
+auto Loc = S.getResumeExpr()->getExprLoc();
+auto *Catch = new (CGF.getContext())
+CXXCatchStmt(Loc, /*exDecl=*/nullptr, Coro.ExceptionHandler);
+auto *TryBody =
+CompoundStmt::Create(CGF.getContext(), S.getResumeExpr(), Loc, Loc);
+TryStmt = CXXTryStmt::Create(CGF.getContext(), Loc, TryBody, Catch);
+CGF.EnterCXXTryStmt(*TryStmt);
+  }
+
   LValueOrRValue Res;
   if (forLValue)
 Res.LV = CGF.EmitLValue(S.getResumeExpr());
   else
 Res.RV = CGF.EmitAnyExpr(S.getResumeExpr(), aggSlot, ignoreResult);
+
+  if (TryStmt) {
+Builder.CreateFlagStore(false, Coro.ResumeEHVar);
+CGF.ExitCXXTryStmt(*TryStmt);
+  }
+
   return Res;
 }
 
@@ -588,19 +618,31 @@ void CodeGenFunction::EmitCoroutineBody(
 EHStack.pushCleanup(EHCleanup);
 
 CurCoro.Data->CurrentAwaitKind = AwaitKind::Init;
+CurCoro.Data->ExceptionHandler = S.getExceptionHandler();
 EmitStmt(S.getInitSuspendStmt());
 CurCoro.Data->FinalJD = getJumpDestInCurrentScope(FinalBB);
 
 CurCoro.Data->CurrentAwaitKind = AwaitKind::Normal;
 
-if (auto *OnException = S.getExceptionHandler()) {
+if (CurCoro.Data->ExceptionHandler) {
+  BasicBlock *BodyBB = createBasicBlock("coro.resumed.body");
+  BasicBlock *ContBB = createBasicBlock("coro.resumed.cont");
+  Value *SkipBody =
+  Builder.CreateFlagLoad(CurCoro.Data->ResumeEHVar, "coro.resumed.eh");
+  Builder.CreateCondBr(SkipBody, ContBB, BodyBB);
+  EmitBlock(BodyBB);
+
   auto Loc = S.getLocStart();
-  CXXCatchStmt Catch(Loc, /*exDecl=*/nullptr, OnException);
-  auto *TryStmt = CXXTryStmt::Create(getContext(), Loc, S.getBody(), 
&Catch);
+  CXXCatchStmt Catch(Loc, /*exDecl=*/nullptr,
+ CurCoro.Data->ExceptionHandler);
+  auto *TryStmt =
+  CXXTryStmt::Create(getContext(), Loc, S.getBody(), &Catch);
 
   EnterCXXTryStmt(*TryStmt);
   emitBodyAndFallthrough(*this, S, TryStmt->getTryBlock());
   ExitCXXTryStmt(*TryStmt);
+
+  EmitBlock(ContBB);
 }
 else {
   emitBodyAndFallthrough(*this, S, S.getBody());

Added: cfe/trunk/test/CodeGenCoroutines/coro-await-resume-eh.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCoroutines/coro-await-resume-eh.cpp?rev=331519&view=auto
==
--- cfe/trunk/test/CodeGenCoroutines/coro-await-resume-eh.cpp (added)
+++ cfe/trunk/test/CodeGenCoroutines/coro-await-resume-eh.cpp Fri May  4 
07:02:37 2018
@@ -0,0 +1,81 @@

[PATCH] D45860: [Coroutines] Catch exceptions in await_resume

[Brian Gesiak via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC331519: [Coroutines] Catch exceptions in await_resume 
(authored by modocache, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D45860?vs=144961&id=145188#toc

Repository:
  rC Clang

https://reviews.llvm.org/D45860

Files:
  lib/CodeGen/CGCoroutine.cpp
  test/CodeGenCoroutines/coro-await-resume-eh.cpp
  test/CodeGenCoroutines/coro-unhandled-exception.cpp

Index: test/CodeGenCoroutines/coro-unhandled-exception.cpp
===
--- test/CodeGenCoroutines/coro-unhandled-exception.cpp
+++ test/CodeGenCoroutines/coro-unhandled-exception.cpp
@@ -48,6 +48,8 @@
 // CHECK: [[CATCHRETDEST]]:
 // CHECK-NEXT: br label %[[TRYCONT:.+]]
 // CHECK: [[TRYCONT]]:
+// CHECK-NEXT: br label %[[RESUMECONT:.+]]
+// CHECK: [[RESUMECONT]]:
 // CHECK-NEXT: br label %[[COROFIN:.+]]
 // CHECK: [[COROFIN]]:
 // CHECK-NEXT: invoke void @"?final_suspend@promise_type@coro_t@@QEAA?AUsuspend_never@coroutines_v1@experimental@std@@XZ"(
@@ -67,6 +69,8 @@
 // CHECK-LPAD: [[CATCHRETDEST]]:
 // CHECK-LPAD-NEXT: br label %[[TRYCONT:.+]]
 // CHECK-LPAD: [[TRYCONT]]:
+// CHECK-LPAD: br label %[[RESUMECONT:.+]]
+// CHECK-LPAD: [[RESUMECONT]]:
 // CHECK-LPAD-NEXT: br label %[[COROFIN:.+]]
 // CHECK-LPAD: [[COROFIN]]:
 // CHECK-LPAD-NEXT: invoke void @_ZN6coro_t12promise_type13final_suspendEv(
Index: test/CodeGenCoroutines/coro-await-resume-eh.cpp
===
--- test/CodeGenCoroutines/coro-await-resume-eh.cpp
+++ test/CodeGenCoroutines/coro-await-resume-eh.cpp
@@ -0,0 +1,81 @@
+// Test the behavior of http://wg21.link/P0664, a proposal to catch any
+// exceptions thrown after the initial suspend point of a coroutine by
+// executing the handler specified by the promise type's 'unhandled_exception'
+// member function.
+//
+// RUN: %clang_cc1 -std=c++14 -fcoroutines-ts \
+// RUN:   -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s \
+// RUN:   -fexceptions -fcxx-exceptions -disable-llvm-passes \
+// RUN:   | FileCheck %s
+
+#include "Inputs/coroutine.h"
+
+namespace coro = std::experimental::coroutines_v1;
+
+struct throwing_awaitable {
+  bool await_ready() { return true; }
+  void await_suspend(coro::coroutine_handle<>) {}
+  void await_resume() { throw 42; }
+};
+
+struct task {
+  struct promise_type {
+task get_return_object() { return task{}; }
+auto initial_suspend() { return throwing_awaitable{}; }
+auto final_suspend() { return coro::suspend_never{}; }
+void return_void() {}
+void unhandled_exception() {}
+  };
+};
+
+// CHECK-LABEL: define void @_Z1fv()
+task f() {
+  // A variable RESUMETHREW is used to keep track of whether the body
+  // of 'await_resume' threw an exception. Exceptions thrown in
+  // 'await_resume' are unwound to RESUMELPAD.
+  // CHECK: init.ready:
+  // CHECK-NEXT: store i1 true, i1* %[[RESUMETHREW:.+]], align 1
+  // CHECK-NEXT: invoke void @_ZN18throwing_awaitable12await_resumeEv
+  // CHECK-NEXT: to label %[[RESUMECONT:.+]] unwind label %[[RESUMELPAD:.+]]
+
+  // If 'await_resume' does not throw an exception, 'false' is stored in
+  // variable RESUMETHREW.
+  // CHECK: [[RESUMECONT]]:
+  // CHECK-NEXT: store i1 false, i1* %[[RESUMETHREW]]
+  // CHECK-NEXT: br label %[[RESUMETRYCONT:.+]]
+
+  // 'unhandled_exception' is called for the exception thrown in
+  // 'await_resume'. The variable RESUMETHREW is never set to false,
+  // and a jump is made to RESUMETRYCONT.
+  // CHECK: [[RESUMELPAD]]:
+  // CHECK: br label %[[RESUMECATCH:.+]]
+  // CHECK: [[RESUMECATCH]]:
+  // CHECK: invoke void @_ZN4task12promise_type19unhandled_exceptionEv
+  // CHECK-NEXT: to label %[[RESUMEENDCATCH:.+]] unwind label
+  // CHECK: [[RESUMEENDCATCH]]:
+  // CHECK-NEXT: invoke void @__cxa_end_catch()
+  // CHECK-NEXT: to label %[[RESUMEENDCATCHCONT:.+]] unwind label
+  // CHECK: [[RESUMEENDCATCHCONT]]:
+  // CHECK-NEXT: br label %[[RESUMETRYCONT]]
+
+  // The variable RESUMETHREW is loaded and if true, then 'await_resume'
+  // threw an exception and the coroutine body is skipped, and the final
+  // suspend is executed immediately. Otherwise, the coroutine body is
+  // executed, and then the final suspend.
+  // CHECK: [[RESUMETRYCONT]]:
+  // CHECK-NEXT: %[[RESUMETHREWLOAD:.+]] = load i1, i1* %[[RESUMETHREW]]
+  // CHECK-NEXT: br i1 %[[RESUMETHREWLOAD]], label %[[RESUMEDCONT:.+]], label %[[RESUMEDBODY:.+]]
+
+  // CHECK: [[RESUMEDBODY]]:
+  // CHECK: invoke void @_ZN4task12promise_type11return_voidEv
+  // CHECK-NEXT: to label %[[REDUMEDBODYCONT:.+]] unwind label
+  // CHECK: [[REDUMEDBODYCONT]]:
+  // CHECK-NEXT: br label %[[COROFINAL:.+]]
+
+  // CHECK: [[RESUMEDCONT]]:
+  // CHECK-NEXT: br label %[[COROFINAL]]
+
+  // CHECK: [[COROFINAL]]:
+  // CHECK-NEXT: invoke void @_ZN4task12promise_type13final_suspendEv
+  co_return;
+}
Index: lib/CodeGen/CGCoroutine.cpp
===

[PATCH] D45860: [Coroutines] Catch exceptions in await_resume

[Brian Gesiak via Phabricator via cfe-commits]

modocache added a comment.

Thanks again for all the reviews, @GorNishanov! Very much appreciated.




Comment at: lib/CodeGen/CGCoroutine.cpp:220
   CGF.EmitBlock(ReadyBlock);
+  CXXTryStmt *TryStmt = nullptr;
+  if (Coro.ExceptionHandler && Kind == AwaitKind::Init) {

GorNishanov wrote:
> I suggest to check whether await_resume expression is noexcept and omit 
> emission of all of the goo. Most of the time the await_ready for 
> initial_suspend will be noexcept and thus we would not need to emit extra 
> stuff.
> 
Oh shoot, I forgot to do this! I'll take care of this in a follow-up commit 
within the next week.


Repository:
  rC Clang

https://reviews.llvm.org/D45860



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

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Alexander Kornienko via Phabricator via cfe-commits]

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

Thank you for the updates. A few more comments.




Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:150
+  } else if (const auto *Try = dyn_cast(St)) {
+auto Uncaught = throwsException(Try->getTryBlock(), Caught);
+for (unsigned i = 0; i < Try->getNumHandlers(); ++i) {

alexfh wrote:
> Please use the concrete type here and below. 
> http://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
>  says 
> 
> > Don’t “almost always” use auto, but do use auto with initializers like 
> > cast(...) or other places where the type is already obvious from the 
> > context.
I don't remember where this comment was, but now I see a few more instances of 
the same issue. Specifically, ThrownExpr and ThrownType here and CaughtType 
below.



Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:17
+
+using namespace clang;
+using namespace clang::ast_matchers;

You can open namespace clang here instead of this using directive.



Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:22
+typedef llvm::SmallVector TypeVec;
+typedef llvm::SmallSet StringSet;
+} // namespace

There's `llvm::StringSet<>` in llvm/ADT/StringSet.h.



Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:25
+
+static const TypeVec throwsException(const FunctionDecl *Func);
+

Too many forward declarations for my taste. Can you just move all static 
functions here and remove unnecessary forward declarations? I guess, one should 
be enough for all the functions here.



Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:178
+}
+auto *NewEnd =
+llvm::remove_if(Uncaught, [&CaughtType](const Type *ThrownType) {

iterator being a pointer is an implementation detail of SmallVector. s/auto 
\*/auto /



Comment at: docs/ReleaseNotes.rst:230
 
+<<< HEAD
+===

Resolve the conflicts.



Comment at: docs/clang-tidy/checks/bugprone-exception-escape.rst:28
+
+   Comma separated list containing function names which should not throw. An
+   example for using this parameter is the function ``WinMain()`` in the

baloghadamsoftware wrote:
> dberris wrote:
> > `EnabledFunctions` but they *should not* throw?
> Maybe we should come up with a better name for this option. I just took this 
> from our company internal check.
FunctionsThatShouldNotThrow?



Comment at: docs/clang-tidy/checks/bugprone-exception-escape.rst:29
+   Comma separated list containing function names which should not throw. An
+   example for using this parameter is the function ``WinMain()`` in the
+   Windows API. Default vale is empty string.

Examples should be copy-pastable. I suppose, one can't use `WinMain()` verbatim 
as the value of this option. An example could be: `main,WinMain` or something 
like that.



Comment at: docs/clang-tidy/checks/bugprone-exception-escape.rst:30
+   example for using this parameter is the function ``WinMain()`` in the
+   Windows API. Default vale is empty string.
+

s/vale/value/


https://reviews.llvm.org/D33537



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

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added inline comments.



Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:154-155
+  if (const auto *TD = ThrownType->getAsTagDecl()) {
+if (TD->getDeclName().isIdentifier() && TD->getName() == "bad_alloc")
+  return Results;
+  }

Does this actually catch `std::bad_alloc` or just any exception called 
`bad_alloc`? Should this be a fully qualified type, those defined by the 
implementation? I suspect this isn't as simple to figure out, because an 
implementation may be using nested namespaces (inline namespace?) to bring in 
the `bad_alloc` type in the `std` namespace.



Comment at: docs/ReleaseNotes.rst:230-233
+<<< HEAD
+===
 - The 'google-runtime-member-string-references' check was removed.
+>>> master

You probably want to fix this...



Comment at: docs/clang-tidy/checks/bugprone-exception-escape.rst:28
+
+   Comma separated list containing function names which should not throw. An
+   example for using this parameter is the function ``WinMain()`` in the

baloghadamsoftware wrote:
> dberris wrote:
> > `EnabledFunctions` but they *should not* throw?
> Maybe we should come up with a better name for this option. I just took this 
> from our company internal check.
My suggestion here would be something like 'FunctionBlacklist' or 
'IgnoreFunctions'.

Are these exact names, or regular expressions? Should they be regular 
expressions? How about those that are in namespaces?

You might want to explore being able to configure this similar to the Sanitizer 
blacklist, which supports function name globs. I can imagine this immediately 
getting really brittle too.



Comment at: test/clang-tidy/bugprone-exception-escape.cpp:178
+void indirect_implicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'indirect_implicit' 
throws
+  implicit_int_thrower();

How deep does this go? Say we have a call to a function that's extern which 
doesn't have 'noexcept' nor a dynamic exception specifier -- do we assume that 
the call to an extern function may throw? Does that warn? What does the warning 
look like? Should it warn? How about when you call a function through a 
function pointer?

The documentation should cover these cases and/or more explicitly say in the 
warning that an exception may throw in a noexcept function (rather than just 
"function <...> throws").


https://reviews.llvm.org/D33537



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

[PATCH] D46393: Remove explicit cfg-temporary-dtors=true

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

In https://reviews.llvm.org/D46393#1086887, @NoQ wrote:

> Thanks!
>
> Just curious - did these flags bother you? Cause we never really care about 
> cleaning up run lines after flipping the flag, so we have a lot of such stale 
> flags in our tests. We could start cleaning them up if they cause problems.


I would like to see this flag removed eventually, since it became default and 
there doesn't seem to be a reason to keep it around. This is just a tiny thing 
to help you with that ;)


Repository:
  rC Clang

https://reviews.llvm.org/D46393



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

[PATCH] D46393: Remove explicit cfg-temporary-dtors=true

[Alexander Kornienko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL331520: Remove explicit cfg-temporary-dtors=true (authored 
by alexfh, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D46393

Files:
  cfe/trunk/test/Analysis/cfg-rich-constructors.cpp
  cfe/trunk/test/Analysis/cfg.cpp
  cfe/trunk/test/Analysis/dtor-cxx11.cpp
  cfe/trunk/test/Analysis/dtor.cpp
  cfe/trunk/test/Analysis/gtest.cpp
  cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp
  cfe/trunk/test/Analysis/temp-obj-dtors-option.cpp


Index: cfe/trunk/test/Analysis/gtest.cpp
===
--- cfe/trunk/test/Analysis/gtest.cpp
+++ cfe/trunk/test/Analysis/gtest.cpp
@@ -1,6 +1,5 @@
 //RUN: %clang_analyze_cc1 -cc1 -std=c++11  
-analyzer-checker=core,apiModeling.google.GTest,debug.ExprInspection 
-analyzer-eagerly-assume %s -verify
 //RUN: %clang_analyze_cc1 -cc1 -std=c++11  
-analyzer-checker=core,apiModeling.google.GTest,debug.ExprInspection 
-analyzer-eagerly-assume -DGTEST_VERSION_1_8_AND_LATER=1 %s -verify
-//RUN: %clang_analyze_cc1 -cc1 -std=c++11  
-analyzer-checker=core,apiModeling.google.GTest,debug.ExprInspection 
-analyzer-eagerly-assume -analyzer-config cfg-temporary-dtors=true %s -verify
 
 void clang_analyzer_eval(int);
 void clang_analyzer_warnIfReached();
Index: cfe/trunk/test/Analysis/temp-obj-dtors-option.cpp
===
--- cfe/trunk/test/Analysis/temp-obj-dtors-option.cpp
+++ cfe/trunk/test/Analysis/temp-obj-dtors-option.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection 
-analyzer-config cfg-temporary-dtors=true,c++-temp-dtor-inlining=false -verify 
%s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection 
-analyzer-config cfg-temporary-dtors=true,c++-temp-dtor-inlining=true -DINLINE 
-verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection 
-analyzer-config c++-temp-dtor-inlining=false -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection 
-analyzer-config c++-temp-dtor-inlining=true -DINLINE -verify %s
 
 void clang_analyzer_eval(bool);
 
Index: cfe/trunk/test/Analysis/cfg.cpp
===
--- cfe/trunk/test/Analysis/cfg.cpp
+++ cfe/trunk/test/Analysis/cfg.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple 
x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++11 
-analyzer-config cfg-rich-constructors=false %s > %t 2>&1
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple 
x86_64-apple-darwin12 -std=c++11 -analyzer-config cfg-rich-constructors=false 
%s > %t 2>&1
 // RUN: FileCheck --input-file=%t -check-prefixes=CHECK,WARNINGS %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple 
x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++11 
-analyzer-config cfg-rich-constructors=true %s > %t 2>&1
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple 
x86_64-apple-darwin12 -std=c++11 -analyzer-config cfg-rich-constructors=true %s 
> %t 2>&1
 // RUN: FileCheck --input-file=%t -check-prefixes=CHECK,ANALYZER %s
 
 // This file tests how we construct two different flavors of the Clang CFG -
Index: cfe/trunk/test/Analysis/cfg-rich-constructors.cpp
===
--- cfe/trunk/test/Analysis/cfg-rich-constructors.cpp
+++ cfe/trunk/test/Analysis/cfg-rich-constructors.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple 
x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++11 -w 
%s > %t 2>&1
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple 
x86_64-apple-darwin12 -std=c++11 -w %s > %t 2>&1
 // RUN: FileCheck --input-file=%t -check-prefixes=CHECK,CXX11 %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple 
x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++17 -w 
%s > %t 2>&1
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple 
x86_64-apple-darwin12 -std=c++17 -w %s > %t 2>&1
 // RUN: FileCheck --input-file=%t -check-prefixes=CHECK,CXX17 %s
 
 class C {
Index: cfe/trunk/test/Analysis/dtor-cxx11.cpp
===
--- cfe/trunk/test/Analysis/dtor-cxx11.cpp
+++ cfe/trunk/test/Analysis/dtor-cxx11.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -std=c++11 
-analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config 
cfg-temporary-dtors=true -Wno-null-dereference -verify %s
+// RUN: %clang_analyze_cc1 -std=c++11 
-analyzer-checker=core,unix.Malloc,debug.ExprInspection -Wno-null-dereference 
-verify %s
 // expected-no-diagnostics
 
 #include "Inputs/system-header-simulator-cxx.h"
Index: cfe/trunk/test/Analysis/dtor.cpp

r331520 - Remove explicit cfg-temporary-dtors=true

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Fri May  4 07:13:14 2018
New Revision: 331520

URL: http://llvm.org/viewvc/llvm-project?rev=331520&view=rev
Log:
Remove explicit cfg-temporary-dtors=true

Summary:
Remove explicit -analyzer-config cfg-temporary-dtors=true in analyzer tests,
since this option defaults to true since r326461.

Reviewers: NoQ

Reviewed By: NoQ

Subscribers: cfe-commits

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

Modified:
cfe/trunk/test/Analysis/cfg-rich-constructors.cpp
cfe/trunk/test/Analysis/cfg.cpp
cfe/trunk/test/Analysis/dtor-cxx11.cpp
cfe/trunk/test/Analysis/dtor.cpp
cfe/trunk/test/Analysis/gtest.cpp
cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp
cfe/trunk/test/Analysis/temp-obj-dtors-option.cpp

Modified: cfe/trunk/test/Analysis/cfg-rich-constructors.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cfg-rich-constructors.cpp?rev=331520&r1=331519&r2=331520&view=diff
==
--- cfe/trunk/test/Analysis/cfg-rich-constructors.cpp (original)
+++ cfe/trunk/test/Analysis/cfg-rich-constructors.cpp Fri May  4 07:13:14 2018
@@ -1,6 +1,6 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple 
x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++11 -w 
%s > %t 2>&1
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple 
x86_64-apple-darwin12 -std=c++11 -w %s > %t 2>&1
 // RUN: FileCheck --input-file=%t -check-prefixes=CHECK,CXX11 %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple 
x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++17 -w 
%s > %t 2>&1
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple 
x86_64-apple-darwin12 -std=c++17 -w %s > %t 2>&1
 // RUN: FileCheck --input-file=%t -check-prefixes=CHECK,CXX17 %s
 
 class C {

Modified: cfe/trunk/test/Analysis/cfg.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cfg.cpp?rev=331520&r1=331519&r2=331520&view=diff
==
--- cfe/trunk/test/Analysis/cfg.cpp (original)
+++ cfe/trunk/test/Analysis/cfg.cpp Fri May  4 07:13:14 2018
@@ -1,6 +1,6 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple 
x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++11 
-analyzer-config cfg-rich-constructors=false %s > %t 2>&1
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple 
x86_64-apple-darwin12 -std=c++11 -analyzer-config cfg-rich-constructors=false 
%s > %t 2>&1
 // RUN: FileCheck --input-file=%t -check-prefixes=CHECK,WARNINGS %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple 
x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++11 
-analyzer-config cfg-rich-constructors=true %s > %t 2>&1
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple 
x86_64-apple-darwin12 -std=c++11 -analyzer-config cfg-rich-constructors=true %s 
> %t 2>&1
 // RUN: FileCheck --input-file=%t -check-prefixes=CHECK,ANALYZER %s
 
 // This file tests how we construct two different flavors of the Clang CFG -

Modified: cfe/trunk/test/Analysis/dtor-cxx11.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/dtor-cxx11.cpp?rev=331520&r1=331519&r2=331520&view=diff
==
--- cfe/trunk/test/Analysis/dtor-cxx11.cpp (original)
+++ cfe/trunk/test/Analysis/dtor-cxx11.cpp Fri May  4 07:13:14 2018
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -std=c++11 
-analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config 
cfg-temporary-dtors=true -Wno-null-dereference -verify %s
+// RUN: %clang_analyze_cc1 -std=c++11 
-analyzer-checker=core,unix.Malloc,debug.ExprInspection -Wno-null-dereference 
-verify %s
 // expected-no-diagnostics
 
 #include "Inputs/system-header-simulator-cxx.h"

Modified: cfe/trunk/test/Analysis/dtor.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/dtor.cpp?rev=331520&r1=331519&r2=331520&view=diff
==
--- cfe/trunk/test/Analysis/dtor.cpp (original)
+++ cfe/trunk/test/Analysis/dtor.cpp Fri May  4 07:13:14 2018
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 
-analyzer-checker=core,unix.Malloc,debug.ExprInspection,cplusplus 
-analyzer-config c++-inlining=destructors,cfg-temporary-dtors=true 
-Wno-null-dereference -Wno-inaccessible-base -verify %s
+// RUN: %clang_analyze_cc1 
-analyzer-checker=core,unix.Malloc,debug.ExprInspection,cplusplus 
-analyzer-config c++-inlining=destructors -Wno-null-dereference 
-Wno-inaccessible-base -verify %s
 
 void clang_analyzer_eval(bool);
 void clang_analyzer_checkInlined(bool);

Modified: cfe/trunk/test/Analysis/gtest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/gtest.cpp?rev=331520&r1=331519&r2=331520&view=diff
===

[PATCH] D41537: Optionally add code completion results for arrow instead of dot

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Sorry for the delay. I really like the direction of this patch!
What's left is defining the semantics of corrections more thoroughly to make 
sure we don't have tricky corner cases that users of the API can't deal with.

PS. This patch is still lacking full context of the diff.
Please use arc 

 or send diff with full context 
.




Comment at: include/clang-c/Index.h:5220
+CINDEX_LINKAGE CXString
+clang_getCompletionCorrection(CXCompletionString completion_string,
+  unsigned correction_index,

I'm a bit vary about exposing source manager and language options in the API 
just for the sake of corrections.
I suggest we add an extra parameter of an existing class 
(`CXCodeCompleteResults`) instead and remove the corresponding helpers to get 
source manager and language options:
`clang_getCompletionNumCorrections(CXCompletionString completion_string, 
CXCodeCompleteResults* results);`




Comment at: include/clang/Sema/CodeCompleteConsumer.h:564
+
+  /// \brief For this completion result correction is required.
+  std::vector Corrections;

Adding some docs here would be useful. These fix-its could be interpreted too 
broadly at this point.
I suggest the following semantics and the comment:

```
/// \brief FixIts that *must* be applied before inserting the text for the 
corresponding completion item.
/// Completion items with non-empty fixits will not be returned by default, 
they should be explicitly requested by setting 
CompletionOptions::IncludeCorrections.
/// For the editors to be able to compute position of the cursor for the 
completion item itself, the following conditions are guaranteed to hold for 
RemoveRange of the stored fixits:
///  - Ranges in the fixits are guaranteed to never contain the completion 
point (or identifier under completion point, if any) inside them, except at the 
start or at the end of the range.
///  - If a fixit range starts or ends with completion point (or starts or ends 
after the identifier under completion point), it will contain at least one 
character. It allows to unambiguously recompute completion point after applying 
the fixit.
/// The intuition is that provided fixits change code around the identifier we 
complete, but are not allowed to touch the identifier itself or the completion 
point.
/// One example of completion items with corrections are the ones replacing '.' 
with '->' and vice versa:
///  std::unique_ptr> vec_ptr;
///  vec_ptr.^  // completion returns an item 'push_back', replacing '.' 
with '->'
///  vec_ptr->^ // completion returns an item 'release', replacing '->' 
with '.'let's put 
```

Do those invariants sound reasonable? Could we add asserts that they hold when 
constructing the completion results?



Comment at: include/clang/Sema/CodeCompleteConsumer.h:565
+  /// \brief For this completion result correction is required.
+  std::vector Corrections;
+

I wonder if we should call them Fixits instead?
To follow the pattern for diagnostics.



Comment at: tools/c-index-test/c-index-test.c:2346
+CXSourceRange correction_range;
+Corr = clang_getCompletionCorrection(completion_result->CompletionString, 
0,
+ source_manager,

We should print all provided fixits in the test code



Comment at: tools/c-index-test/c-index-test.c:2350
+ &correction_range);
+fprintf(file, " (requires correction to \"%s\")", clang_getCString(Corr));
+  }

maybe add a source range or source of the replaced text to the printf here?
e.g. "`replaces '.' with '->'`"?



Comment at: tools/libclang/CIndexCodeCompletion.cpp:340
+  const FixItHint &Correction = Corrections[correction_index];
+  if (replacement_range) {
+SourceManager *SourceMgr = (SourceManager *)source_manager;

Maybe return a struct containing both the string and CXSourceRange instead of 
this output parameter?
It does not make sense to get a string without the corresponding source range.
How does libclang return fixits for diagnostics? We probably want to follow the 
same pattern here.


https://reviews.llvm.org/D41537



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

[PATCH] D46180: [clang-format] Refactor #include insertion/deletion functionality into a class.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

I somehow missed the review email, sorry for the delayed response.

Just one nit and one question from me on changed behavior in the tests (quoted 
vs angled #include).
Otherwise LG, just wanted to make sure the change in behavior is intentional.




Comment at: lib/Format/Format.cpp:2131
+inline StringRef trimInclude(StringRef IncludeName) {
+  return IncludeName.trim("\"<>");
+}

NIT: Maybe we could add some asserts to this function that the passed include 
name is actually properly quoted.
E.g. starts with '<' or '"', ends with the corresponding char, etc.

So long as this is part of `Format.cpp`, it's not terribly important, since 
it's only called on paths that come from regex matches.
If we make it a public later, having asserts would be really useful.
That said, we might address it later when/if we actually make this function 
public.



Comment at: unittests/Format/CleanupTest.cpp:862
   std::string Expected = "#include \"a.h\"\n"
- "#include \"vector\"\n"
- "#include \n"
- "#include \n";
+ "#include \n";
   tooling::Replacements Replaces =

Are we sure we want this behavior change?
The API seems to allow us keeping the old one.

I know there's a FIXME there, but I just wanted to understand better what's the 
right thing to do here and why.


Repository:
  rC Clang

https://reviews.llvm.org/D46180



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

[PATCH] D46394: [clang-cl] Print /showIncludes to stderr, if used in combination with /E, /EP or /P

[Felix Bruns via Phabricator via cfe-commits]

fxb updated this revision to Diff 145196.
fxb added a comment.

1. Fixed the compile error caused by re-using the name 
`ShowIncludesDestination`. The member variable is now named `ShowIncludesDest`.
2. Fixed `test/Frontend/print-header-includes.c` to test both cases:
  - If only `--show-includes` is passed, includes are printed on stdout.
  - If  both `--show-includes` and `-E` are passed, includes are printed on 
stderr.




https://reviews.llvm.org/D46394

Files:
  include/clang/Frontend/DependencyOutputOptions.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInstance.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/HeaderIncludeGen.cpp
  test/Driver/cl-options.c
  test/Frontend/print-header-includes.c

Index: test/Frontend/print-header-includes.c
===
--- test/Frontend/print-header-includes.c
+++ test/Frontend/print-header-includes.c
@@ -5,16 +5,24 @@
 // CHECK: . {{.*test.h}}
 // CHECK: .. {{.*test2.h}}
 
-// RUN: %clang_cc1 -I%S -include Inputs/test3.h -E --show-includes -o /dev/null %s | \
-// RUN: FileCheck --strict-whitespace --check-prefix=MS %s
-// MS-NOT: 
-// MS: Note: including file: {{[^ ]*test3.h}}
-// MS: Note: including file: {{[^ ]*test.h}}
-// MS: Note: including file:  {{[^ ]*test2.h}}
-// MS-NOT: Note
+// RUN: %clang_cc1 -I%S -include Inputs/test3.h --show-includes -o /dev/null %s | \
+// RUN: FileCheck --strict-whitespace --check-prefix=MS-STDOUT %s
+// MS-STDOUT-NOT: 
+// MS-STDOUT: Note: including file: {{[^ ]*test3.h}}
+// MS-STDOUT: Note: including file: {{[^ ]*test.h}}
+// MS-STDOUT: Note: including file:  {{[^ ]*test2.h}}
+// MS-STDOUT-NOT: Note
+
+// RUN: %clang_cc1 -I%S -include Inputs/test3.h -E --show-includes -o /dev/null %s 2> %t.stderr
+// RUN: FileCheck --strict-whitespace --check-prefix=MS-STDERR < %t.stderr %s
+// MS-STDERR-NOT: 
+// MS-STDERR: Note: including file: {{[^ ]*test3.h}}
+// MS-STDERR: Note: including file: {{[^ ]*test.h}}
+// MS-STDERR: Note: including file:  {{[^ ]*test2.h}}
+// MS-STDERR-NOT: Note
 
 // RUN: echo "fun:foo" > %t.blacklist
-// RUN: %clang_cc1 -I%S -fsanitize=address -fdepfile-entry=%t.blacklist -E --show-includes -o /dev/null %s | \
+// RUN: %clang_cc1 -I%S -fsanitize=address -fdepfile-entry=%t.blacklist --show-includes -o /dev/null %s | \
 // RUN: FileCheck --strict-whitespace --check-prefix=MS-BLACKLIST %s
 // MS-BLACKLIST: Note: including file: {{[^ ]*\.blacklist}}
 // MS-BLACKLIST: Note: including file: {{[^ ]*test.h}}
Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -198,10 +198,8 @@
 // RUN: %clang_cl /E /showIncludes -### -- %s 2>&1 | FileCheck -check-prefix=showIncludes_E %s
 // RUN: %clang_cl /EP /showIncludes -### -- %s 2>&1 | FileCheck -check-prefix=showIncludes_E %s
 // RUN: %clang_cl /E /EP /showIncludes -### -- %s 2>&1 | FileCheck -check-prefix=showIncludes_E %s
-// showIncludes_E: warning: argument unused during compilation: '--show-includes'
-
-// RUN: %clang_cl /EP /P /showIncludes -### -- %s 2>&1 | FileCheck -check-prefix=showIncludes_E_And_P %s
-// showIncludes_E_And_P-NOT: warning: argument unused during compilation: '--show-includes'
+// RUN: %clang_cl /EP /P /showIncludes -### -- %s 2>&1 | FileCheck -check-prefix=showIncludes_E %s
+// showIncludes_E-NOT: warning: argument unused during compilation: '--show-includes'
 
 // /source-charset: should warn on everything except UTF-8.
 // RUN: %clang_cl /source-charset:utf-16 -### -- %s 2>&1 | FileCheck -check-prefix=source-charset-utf-16 %s
Index: lib/Frontend/HeaderIncludeGen.cpp
===
--- lib/Frontend/HeaderIncludeGen.cpp
+++ lib/Frontend/HeaderIncludeGen.cpp
@@ -80,9 +80,23 @@
const DependencyOutputOptions &DepOpts,
bool ShowAllHeaders, StringRef OutputPath,
bool ShowDepth, bool MSStyle) {
-  raw_ostream *OutputFile = MSStyle ? &llvm::outs() : &llvm::errs();
+  raw_ostream *OutputFile = &llvm::errs();
   bool OwnsOutputFile = false;
 
+  // Choose output stream, when printing in cl.exe /showIncludes style.
+  if (MSStyle) {
+switch (DepOpts.ShowIncludesDest) {
+default:
+  llvm_unreachable("Invalid destination for /showIncludes output!");
+case ShowIncludesDestination::Stderr:
+  OutputFile = &llvm::errs();
+  break;
+case ShowIncludesDestination::Stdout:
+  OutputFile = &llvm::outs();
+  break;
+}
+  }
+
   // Open the output file, if used.
   if (!OutputPath.empty()) {
 std::error_code EC;
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1130,7 +1130,17 @@
   Opts.ShowHeaderIncludes = Args.hasArg(OPT_H);
   Opts.H

[PATCH] D46435: [x86] Introduce the encl[u|s|v] intrinsics

[Gabor Buella via Phabricator via cfe-commits]

GBuella created this revision.
GBuella added reviewers: craig.topper, zvi.
Herald added subscribers: cfe-commits, mgorny.

Repository:
  rC Clang

https://reviews.llvm.org/D46435

Files:
  lib/Headers/CMakeLists.txt
  lib/Headers/module.modulemap
  lib/Headers/sgxintrin.h
  lib/Headers/x86intrin.h
  test/Headers/sgxintrin.c

Index: test/Headers/sgxintrin.c
===
--- /dev/null
+++ test/Headers/sgxintrin.c
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 %s -ffreestanding -triple x86_64-unknown-unknown -target-feature +sgx -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-64
+// RUN: %clang_cc1 %s -ffreestanding -triple i386 -target-feature +sgx -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-32
+
+#include 
+#include 
+#include 
+
+uint32_t test_encls(uint32_t leaf, size_t data[3]) {
+// CHECK-64: call { i32, i64, i64, i64 } asm "encls", "={ax},={bx},={cx},={dx},{ax},{bx},{cx},{dx},~{cc},~{dirflag},~{fpsr},~{flags}"(i32 %{{.*}}, i64 %{{.*}}, i64 %{{.*}}, i64 %{{.*}})
+// CHECK-32: call { i32, i32, i32, i32 } asm "encls", "={ax},={bx},={cx},={dx},{ax},{bx},{cx},{dx},~{cc},~{dirflag},~{fpsr},~{flags}"(i32 %{{.*}}, i32 %{{.*}}, i32 %{{.*}}, i32 %{{.*}})
+
+  return _encls_u32(leaf, data);
+}
+
+uint32_t test_enclu(uint32_t leaf, size_t data[3]) {
+// CHECK-64: call { i32, i64, i64, i64 } asm "enclu", "={ax},={bx},={cx},={dx},{ax},{bx},{cx},{dx},~{cc},~{dirflag},~{fpsr},~{flags}"(i32 %{{.*}}, i64 %{{.*}}, i64 %{{.*}}, i64 %{{.*}})
+// CHECK-32: call { i32, i32, i32, i32 } asm "enclu", "={ax},={bx},={cx},={dx},{ax},{bx},{cx},{dx},~{cc},~{dirflag},~{fpsr},~{flags}"(i32 %{{.*}}, i32 %{{.*}}, i32 %{{.*}}, i32 %{{.*}})
+
+  return _enclu_u32(leaf, data);
+}
+
+uint32_t test_enclv(uint32_t leaf, size_t data[3]) {
+// CHECK-64: call { i32, i64, i64, i64 } asm "enclv", "={ax},={bx},={cx},={dx},{ax},{bx},{cx},{dx},~{cc},~{dirflag},~{fpsr},~{flags}"(i32 %{{.*}}, i64 %{{.*}}, i64 %{{.*}}, i64 %{{.*}})
+// CHECK-32: call { i32, i32, i32, i32 } asm "enclv", "={ax},={bx},={cx},={dx},{ax},{bx},{cx},{dx},~{cc},~{dirflag},~{fpsr},~{flags}"(i32 %{{.*}}, i32 %{{.*}}, i32 %{{.*}}, i32 %{{.*}})
+
+  return _enclv_u32(leaf, data);
+}
Index: lib/Headers/x86intrin.h
===
--- lib/Headers/x86intrin.h
+++ lib/Headers/x86intrin.h
@@ -105,4 +105,8 @@
 #include 
 #endif
 
+#if !defined(_MSC_VER) || __has_feature(modules) || defined(__SGX__)
+#include 
+#endif
+
 #endif /* __X86INTRIN_H */
Index: lib/Headers/sgxintrin.h
===
--- /dev/null
+++ lib/Headers/sgxintrin.h
@@ -0,0 +1,70 @@
+/*=== sgxintrin.h - X86 SGX intrinsics configuration ---===
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ *===---===
+ */
+
+#ifndef __X86INTRIN_H
+#error "Never use  directly; include  instead."
+#endif
+
+#ifndef __SGXINTRIN_H
+#define __SGXINTRIN_H
+
+/* Define the default attributes for the functions in this file. */
+#define __DEFAULT_FN_ATTRS \
+  __attribute__((__always_inline__, __nodebug__,  __target__("sgx")))
+
+static __inline unsigned int __DEFAULT_FN_ATTRS
+_enclu_u32(unsigned int __leaf, __SIZE_TYPE__ __d[])
+{
+  unsigned int __result;
+  __asm__ ("enclu"
+   : "=a" (__result), "=b" (__d[0]), "=c" (__d[1]), "=d" (__d[2])
+   : "a" (__leaf), "b" (__d[0]), "c" (__d[1]), "d" (__d[2])
+   : "cc");
+  return __result;
+}
+
+static __inline unsigned int __DEFAULT_FN_ATTRS
+_encls_u32(unsigned int __leaf, __SIZE_TYPE__ __d[])
+{
+  unsigned int __result;
+  __asm__ ("encls"
+   : "=a" (__result), "=b" (__d[0]), "=c" (__d[1]), "=d" (__d[2])
+   : "a" (__leaf), "b" (__d[0]), "c" (__d[1]), "d" (__d[2])
+   : "cc");
+  return __result;
+}
+
+static __inline unsigned int __DEFAULT_FN_ATTRS
+_enclv_u32(unsigned int

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin added a comment.

Hi Rafael! I like the change.




Comment at: lib/CrossTU/CrossTranslationUnit.cpp:276
+  ASTImporter &Importer = getOrCreateASTImporter(D->getASTContext());
+  auto *ToDecl = cast(Importer.Import(const_cast(D)));
+  assert(HasDefinition(ToDecl));

Are we always sure that Import() returns non-null value of appropriate type? If 
no, we have to check the result after applying dyn_cast_or_null.


Repository:
  rC Clang

https://reviews.llvm.org/D46421



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

[PATCH] D46431: [x86] Introduce the pconfig intrinsic

[Gabor Buella via Phabricator via cfe-commits]

GBuella updated this revision to Diff 145207.

https://reviews.llvm.org/D46431

Files:
  include/clang/Driver/Options.td
  lib/Basic/Targets/X86.cpp
  lib/Basic/Targets/X86.h
  lib/Headers/CMakeLists.txt
  lib/Headers/cpuid.h
  lib/Headers/module.modulemap
  lib/Headers/pconfigintrin.h
  lib/Headers/x86intrin.h
  test/Driver/x86-target-features.c
  test/Headers/pconfigintin.c
  test/Preprocessor/predefined-arch-macros.c

Index: test/Preprocessor/predefined-arch-macros.c
===
--- test/Preprocessor/predefined-arch-macros.c
+++ test/Preprocessor/predefined-arch-macros.c
@@ -1204,6 +1204,7 @@
 // CHECK_ICX_M32: #define __MMX__ 1
 // CHECK_ICX_M32: #define __MPX__ 1
 // CHECK_ICX_M32: #define __PCLMUL__ 1
+// CHECK_ICX_M32: #define __PCONFIG__ 1
 // CHECK_ICX_M32: #define __PKU__ 1
 // CHECK_ICX_M32: #define __POPCNT__ 1
 // CHECK_ICX_M32: #define __PRFCHW__ 1
@@ -1261,6 +1262,7 @@
 // CHECK_ICX_M64: #define __MMX__ 1
 // CHECK_ICX_M64: #define __MPX__ 1
 // CHECK_ICX_M64: #define __PCLMUL__ 1
+// CHECK_ICX_M64: #define __PCONFIG__ 1
 // CHECK_ICX_M64: #define __PKU__ 1
 // CHECK_ICX_M64: #define __POPCNT__ 1
 // CHECK_ICX_M64: #define __PRFCHW__ 1
Index: test/Headers/pconfigintin.c
===
--- /dev/null
+++ test/Headers/pconfigintin.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 %s -ffreestanding -triple x86_64-unknown-unknown -target-feature +pconfig -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-64
+// RUN: %clang_cc1 %s -ffreestanding -triple i386 -target-feature +pconfig -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-32
+
+#include 
+#include 
+#include 
+
+uint32_t test_pconfig(uint32_t leaf, size_t data[3]) {
+// CHECK-64: call { i32, i64, i64, i64 } asm "pconfig", "={ax},={bx},={cx},={dx},{ax},{bx},{cx},{dx},~{cc},~{dirflag},~{fpsr},~{flags}"(i32 %{{.*}}, i64 %{{.*}}, i64 %{{.*}}, i64 %{{.*}})
+// CHECK-32: call { i32, i32, i32, i32 } asm "pconfig", "={ax},={bx},={cx},={dx},{ax},{bx},{cx},{dx},~{cc},~{dirflag},~{fpsr},~{flags}"(i32 %{{.*}}, i32 %{{.*}}, i32 %{{.*}}, i32 %{{.*}})
+  return _pconfig_u32(leaf, data);
+}
Index: test/Driver/x86-target-features.c
===
--- test/Driver/x86-target-features.c
+++ test/Driver/x86-target-features.c
@@ -159,3 +159,8 @@
 // RUN: %clang -target i386-unknown-linux-gnu -march=i386 -mno-movdir64b %s -### -o %t.o 2>&1 | FileCheck -check-prefix=NO-MOVDIR64B %s
 // MOVDIR64B: "-target-feature" "+movdir64b"
 // NO-MOVDIR64B: "-target-feature" "-movdir64b"
+
+// RUN: %clang -target i386-unknown-linux-gnu -march=i386 -mpconfig %s -### -o %t.o 2>&1 | FileCheck -check-prefix=PCONFIG %s
+// RUN: %clang -target i386-unknown-linux-gnu -march=i386 -mno-pconfig %s -### -o %t.o 2>&1 | FileCheck -check-prefix=NO-PCONFIG %s
+// PCONFIG: "-target-feature" "+pconfig"
+// NO-PCONFIG: "-target-feature" "-pconfig"
Index: lib/Headers/x86intrin.h
===
--- lib/Headers/x86intrin.h
+++ lib/Headers/x86intrin.h
@@ -105,4 +105,8 @@
 #include 
 #endif
 
+#if !defined(_MSC_VER) || __has_feature(modules) || defined(__PCONFIG__)
+#include 
+#endif
+
 #endif /* __X86INTRIN_H */
Index: lib/Headers/pconfigintrin.h
===
--- /dev/null
+++ lib/Headers/pconfigintrin.h
@@ -0,0 +1,50 @@
+/*=== pconfigintrin.h - X86 platform configuration -===
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ *===---===
+ */
+
+#ifndef __X86INTRIN_H
+#error "Never use  directly; include  instead."
+#endif
+
+#ifndef __PCONFIGINTRIN_H
+#define __PCONFIGINTRIN_H
+
+#define __PCONFIG_KEY_PROGRAM 0x0001
+
+/* Define the default attributes for the functions in this file. */
+#define __DEFAULT_FN_ATTRS \

r331533 - [clang-cl] Print /showIncludes to stderr, if used in combination with /E, /EP or /P

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Fri May  4 08:58:31 2018
New Revision: 331533

URL: http://llvm.org/viewvc/llvm-project?rev=331533&view=rev
Log:
[clang-cl] Print /showIncludes to stderr, if used in combination with /E, /EP 
or /P

This replicates 'cl.exe' behavior and allows for both preprocessor output and
dependency information to be extraced with a single compiler invocation.

This is especially useful for compiler caching with tools like Mozilla's 
sccache.

See: https://github.com/mozilla/sccache/issues/246

Patch By: fxb

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

Modified:
cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CompilerInstance.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Frontend/HeaderIncludeGen.cpp
cfe/trunk/test/Driver/cl-options.c
cfe/trunk/test/Frontend/print-header-includes.c

Modified: cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h?rev=331533&r1=331532&r2=331533&view=diff
==
--- cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h (original)
+++ cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h Fri May  4 
08:58:31 2018
@@ -15,6 +15,9 @@
 
 namespace clang {
 
+/// ShowIncludesDestination - Destination for /showIncludes output.
+enum class ShowIncludesDestination { None, Stdout, Stderr };
+
 /// DependencyOutputFormat - Format for the compiler dependency file.
 enum class DependencyOutputFormat { Make, NMake };
 
@@ -28,9 +31,11 @@ public:
  /// dependency, which can avoid some 
'make'
  /// problems.
   unsigned AddMissingHeaderDeps : 1; ///< Add missing headers to dependency 
list
-  unsigned PrintShowIncludes : 1; ///< Print cl.exe style /showIncludes info.
   unsigned IncludeModuleFiles : 1; ///< Include module file dependencies.
 
+  /// Destination of cl.exe style /showIncludes info.
+  ShowIncludesDestination ShowIncludesDest;
+
   /// The format for the dependency file.
   DependencyOutputFormat OutputFormat;
 
@@ -65,8 +70,8 @@ public:
 ShowHeaderIncludes = 0;
 UsePhonyTargets = 0;
 AddMissingHeaderDeps = 0;
-PrintShowIncludes = 0;
 IncludeModuleFiles = 0;
+ShowIncludesDest = ShowIncludesDestination::None;
 OutputFormat = DependencyOutputFormat::Make;
   }
 };

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=331533&r1=331532&r2=331533&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Fri May  4 08:58:31 2018
@@ -5082,13 +5082,8 @@ void Clang::AddClangCLArgs(const ArgList
 CmdArgs.push_back("--dependent-lib=oldnames");
   }
 
-  // Both /showIncludes and /E (and /EP) write to stdout. Allowing both
-  // would produce interleaved output, so ignore /showIncludes in such cases.
-  if ((!Args.hasArg(options::OPT_E) && !Args.hasArg(options::OPT__SLASH_EP)) ||
-  (Args.hasArg(options::OPT__SLASH_P) &&
-   Args.hasArg(options::OPT__SLASH_EP) && !Args.hasArg(options::OPT_E)))
-if (Arg *A = Args.getLastArg(options::OPT_show_includes))
-  A->render(Args, CmdArgs);
+  if (Arg *A = Args.getLastArg(options::OPT_show_includes))
+A->render(Args, CmdArgs);
 
   // This controls whether or not we emit RTTI data for polymorphic types.
   if (Args.hasFlag(options::OPT__SLASH_GR_, options::OPT__SLASH_GR,

Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=331533&r1=331532&r2=331533&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Fri May  4 08:58:31 2018
@@ -462,7 +462,7 @@ void CompilerInstance::createPreprocesso
/*ShowDepth=*/false);
   }
 
-  if (DepOpts.PrintShowIncludes) {
+  if (DepOpts.ShowIncludesDest != ShowIncludesDestination::None) {
 AttachHeaderIncludeGen(*PP, DepOpts,
/*ShowAllHeaders=*/true, /*OutputPath=*/"",
/*ShowDepth=*/true, /*MSStyle=*/true);

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=331533&r1=331532&r2=331533&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Fri May  4 08:58:31 2018
@@ -1130,7 +1130,17 @@ static void Pars

[PATCH] D46180: [clang-format] Refactor #include insertion/deletion functionality into a class.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 145210.
ioeric marked an inline comment as done.
ioeric added a comment.

- Addressed review comment.


Repository:
  rC Clang

https://reviews.llvm.org/D46180

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

Index: unittests/Format/CleanupTest.cpp
===
--- unittests/Format/CleanupTest.cpp
+++ unittests/Format/CleanupTest.cpp
@@ -471,19 +471,77 @@
   EXPECT_EQ(Expected, apply(Code, Replaces));
 }
 
+TEST_F(CleanUpReplacementsTest, InsertIntoBlockSorted) {
+  std::string Code = "#include \"x/fix.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \n";
+  std::string Expected = "#include \"x/fix.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"b.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \n";
+  tooling::Replacements Replaces =
+  toReplacements({createInsertion("#include \"b.h\"")});
+  EXPECT_EQ(Expected, apply(Code, Replaces));
+}
+
+TEST_F(CleanUpReplacementsTest, InsertIntoFirstBlockOfSameKind) {
+  std::string Code = "#include \"x/fix.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \"e.h\"\n"
+ "#include \"f.h\"\n"
+ "#include \n"
+ "#include \n"
+ "#include \"m.h\"\n"
+ "#include \"n.h\"\n";
+  std::string Expected = "#include \"x/fix.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \"d.h\"\n"
+ "#include \"e.h\"\n"
+ "#include \"f.h\"\n"
+ "#include \n"
+ "#include \n"
+ "#include \"m.h\"\n"
+ "#include \"n.h\"\n";
+  tooling::Replacements Replaces =
+  toReplacements({createInsertion("#include \"d.h\"")});
+  EXPECT_EQ(Expected, apply(Code, Replaces));
+}
+
+TEST_F(CleanUpReplacementsTest, InsertIntoSystemBlockSorted) {
+  std::string Code = "#include \"x/fix.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \n"
+ "#include \n";
+  std::string Expected = "#include \"x/fix.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \n"
+ "#include \n"
+ "#include \n";
+  tooling::Replacements Replaces =
+  toReplacements({createInsertion("#include ")});
+  EXPECT_EQ(Expected, apply(Code, Replaces));
+}
+
+
 TEST_F(CleanUpReplacementsTest, InsertMultipleIncludesLLVMStyle) {
   std::string Code = "#include \"x/fix.h\"\n"
  "#include \"a.h\"\n"
  "#include \"b.h\"\n"
+ "#include \"z.h\"\n"
  "#include \"clang/Format/Format.h\"\n"
  "#include \n";
   std::string Expected = "#include \"x/fix.h\"\n"
  "#include \"a.h\"\n"
  "#include \"b.h\"\n"
  "#include \"new/new.h\"\n"
+ "#include \"z.h\"\n"
  "#include \"clang/Format/Format.h\"\n"
- "#include \n"
- "#include \n";
+ "#include \n"
+ "#include \n";
   tooling::Replacements Replaces =
   toReplacements({createInsertion("#include "),
   createInsertion("#include \"new/new.h\"")});
@@ -517,12 +575,12 @@
  "#include \"z/b.h\"\n";
   std::string Expected = "#include \"x/fix.h\"\n"
  "\n"
- "#include \n"
  "#include \n"
+ "#include \n"
  "\n"
+ "#include \"x/x.h\"\n"
  "#include \"y/a.h\"\n"
- "#include \"z/b.h\"\n"
- "#include \"x/x.h\"\n";
+ "#include \"z/b.h\"\n";
   tooling::Replacements Replaces =
   toReplacements({createInsertion("#include "),
   createInsertion("#include \"x/x.h\"")});
@@ -776,8 +834,10 @@
 
 TEST_F(CleanUpReplacementsTest, NoNewLineAtTheEndOfCodeMultipleInsertions) {
   std::string Code = "#include ";
+  // FIXME: a better behavior is to only append on newline to Code, but this
+  // case should be rare in practice.
   std::string Expected =
-  "#include \n#include \n#include \n";
+  "#include \n#include \n\n#include \n";
   tooling::Replacements Replaces =
   toReplacements({createInsertion("#include "),
   createInsertion("#include ")});
@@ -801,8 +861,8 @@
   // FIXME: th

[PATCH] D36610: [Tooling] Add option to getFullyQualifiedName using a custom PritingPolicy

[Mikhail Ramalho via Phabricator via cfe-commits]

mikhail.ramalho added a comment.

Thanks.

I just need someone to push it, as I don't have write access to the repo.


https://reviews.llvm.org/D36610



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

r331536 - [NFC]Convert Class to use member initialization instead of inline.

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Fri May  4 09:19:53 2018
New Revision: 331536

URL: http://llvm.org/viewvc/llvm-project?rev=331536&view=rev
Log:
[NFC]Convert Class to use member initialization instead of inline.

Modified:
cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h

Modified: cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h?rev=331536&r1=331535&r2=331536&view=diff
==
--- cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h (original)
+++ cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h Fri May  4 
09:19:53 2018
@@ -65,15 +65,11 @@ public:
   std::string ModuleDependencyOutputDir;
 
 public:
-  DependencyOutputOptions() {
-IncludeSystemHeaders = 0;
-ShowHeaderIncludes = 0;
-UsePhonyTargets = 0;
-AddMissingHeaderDeps = 0;
-IncludeModuleFiles = 0;
-ShowIncludesDest = ShowIncludesDestination::None;
-OutputFormat = DependencyOutputFormat::Make;
-  }
+  DependencyOutputOptions()
+  : IncludeSystemHeaders(0), ShowHeaderIncludes(0), UsePhonyTargets(0),
+AddMissingHeaderDeps(0), IncludeModuleFiles(0),
+ShowIncludesDest(ShowIncludesDestination::None),
+OutputFormat(DependencyOutputFormat::Make) {}
 };
 
 }  // end namespace clang


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

[PATCH] D46374: Add support for ObjC property name to be a single acronym.

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton accepted this revision.
benhamilton added inline comments.
This revision is now accepted and ready to land.



Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:222
+  [MatchedDecl](std::string const &s) {
+auto Acronym  = llvm::Regex("^" + s + "$");
+return Acronym.match(MatchedDecl->getName());

Please be aware this will change the match from running a single regular 
expression to running ~ 70 regular expressions on every single `@property`. I 
would expect this to perform pretty poorly.




Comment at: test/clang-tidy/objc-property-declaration.m:24
 @property(assign, nonatomic) int enableGLAcceleration;
+@property(assign, nonatomic) int ID;
 @end

Wizard wrote:
> benhamilton wrote:
> > Please add a test for a built-in regex (4G) as well as a custom regex in 
> > the other test file.
> Unable to add single property test of 4G because it is illegal to use digit 
> as the first character of property name.
Ah, of course.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46374



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

[PATCH] D46439: Fix incorrect packed aligned structure layout

[Momchil Velikov via Phabricator via cfe-commits]

chill created this revision.
chill added reviewers: rsmith, aaron.ballman.

Executing the following program

  #include 
  #include 
  
  struct S {
char x;
int y;
  } __attribute__((packed, aligned(8)));
  
  struct alignas(8) T {
char x;
int y;
  } __attribute__((packed));
  
  int main() {
assert(offsetof(S, x) == 0);
assert(offsetof(S, y) == 1);
  
assert(offsetof(T, x) == 0);
assert(offsetof(T, y) == 1);
  }

fails with assertion

  a.out: a.cc:19: int main(): Assertion `offsetof(T, y) == 1' failed.

The layout if `T` is incorrect, because it's computed and effectively cached 
when checking for `alignas` under-aligning the structure, however, this happens 
before `__attribute__((packed))` is processed.

This patch moves the processing of attributes before the `alignas` 
under-alignment check.


Repository:
  rC Clang

https://reviews.llvm.org/D46439

Files:
  lib/Sema/SemaDecl.cpp


Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -15573,6 +15573,10 @@
 if (!Completed)
   Record->completeDefinition();
 
+// Handle attributes before checking for alignas underalignment.
+if (Attr)
+  ProcessDeclAttributeList(S, Record, Attr);
+
 // We may have deferred checking for a deleted destructor. Check now.
 if (CXXRecordDecl *CXXRecord = dyn_cast(Record)) {
   auto *Dtor = CXXRecord->getDestructor();
@@ -15703,9 +15707,6 @@
   CDecl->setIvarRBraceLoc(RBrac);
 }
   }
-
-  if (Attr)
-ProcessDeclAttributeList(S, Record, Attr);
 }
 
 /// \brief Determine whether the given integral value is representable within


Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -15573,6 +15573,10 @@
 if (!Completed)
   Record->completeDefinition();
 
+// Handle attributes before checking for alignas underalignment.
+if (Attr)
+  ProcessDeclAttributeList(S, Record, Attr);
+
 // We may have deferred checking for a deleted destructor. Check now.
 if (CXXRecordDecl *CXXRecord = dyn_cast(Record)) {
   auto *Dtor = CXXRecord->getDestructor();
@@ -15703,9 +15707,6 @@
   CDecl->setIvarRBraceLoc(RBrac);
 }
   }
-
-  if (Attr)
-ProcessDeclAttributeList(S, Record, Attr);
 }
 
 /// \brief Determine whether the given integral value is representable within
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46180: [clang-format] Refactor #include insertion/deletion functionality into a class.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D46180



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

[clang-tools-extra] r331539 - [clang-doc] Attaching a name to reference data

[Julie Hockett via cfe-commits]

Author: juliehockett
Date: Fri May  4 10:02:13 2018
New Revision: 331539

URL: http://llvm.org/viewvc/llvm-project?rev=331539&view=rev
Log:
[clang-doc] Attaching a name to reference data

This adds the name of the referenced decl, in addition to its USR, to
the saved data, so that the backend can look at an info in isolation and
still be able to construct a human-readable name for it.

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

Modified:
clang-tools-extra/trunk/clang-doc/BitcodeWriter.cpp
clang-tools-extra/trunk/clang-doc/BitcodeWriter.h
clang-tools-extra/trunk/clang-doc/Representation.h
clang-tools-extra/trunk/clang-doc/Serialize.cpp
clang-tools-extra/trunk/test/clang-doc/mapper-class-in-class.cpp
clang-tools-extra/trunk/test/clang-doc/mapper-class-in-function.cpp
clang-tools-extra/trunk/test/clang-doc/mapper-class.cpp
clang-tools-extra/trunk/test/clang-doc/mapper-comments.cpp
clang-tools-extra/trunk/test/clang-doc/mapper-enum.cpp
clang-tools-extra/trunk/test/clang-doc/mapper-function.cpp
clang-tools-extra/trunk/test/clang-doc/mapper-method.cpp
clang-tools-extra/trunk/test/clang-doc/mapper-namespace.cpp
clang-tools-extra/trunk/test/clang-doc/mapper-struct.cpp
clang-tools-extra/trunk/test/clang-doc/mapper-union.cpp

Modified: clang-tools-extra/trunk/clang-doc/BitcodeWriter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-doc/BitcodeWriter.cpp?rev=331539&r1=331538&r2=331539&view=diff
==
--- clang-tools-extra/trunk/clang-doc/BitcodeWriter.cpp (original)
+++ clang-tools-extra/trunk/clang-doc/BitcodeWriter.cpp Fri May  4 10:02:13 2018
@@ -14,6 +14,9 @@
 namespace clang {
 namespace doc {
 
+// Empty SymbolID for comparison, so we don't have to construct one every time.
+static const SymbolID EmptySID = SymbolID();
+
 // Since id enums are not zero-indexed, we need to transform the given id into
 // its associated index.
 struct BlockIdToIndexFunctor {
@@ -82,18 +85,6 @@ static void LocationAbbrev(std::shared_p
llvm::BitCodeAbbrevOp(llvm::BitCodeAbbrevOp::Blob)});
 }
 
-static void ReferenceAbbrev(std::shared_ptr &Abbrev) {
-  AbbrevGen(Abbrev,
-{// 0. Fixed-size integer (ref type)
- llvm::BitCodeAbbrevOp(llvm::BitCodeAbbrevOp::Fixed,
-   BitCodeConstants::ReferenceTypeSize),
- // 1. Fixed-size integer (length of the USR or UnresolvedName)
- llvm::BitCodeAbbrevOp(llvm::BitCodeAbbrevOp::Fixed,
-   BitCodeConstants::StringLengthSize),
- // 2. The string blob
- llvm::BitCodeAbbrevOp(llvm::BitCodeAbbrevOp::Blob)});
-}
-
 struct RecordIdDsc {
   llvm::StringRef Name;
   AbbrevDsc Abbrev = nullptr;
@@ -124,7 +115,8 @@ static const llvm::IndexedMap StringUSR;
-  StringRef OutString;
-  if (Ref.RefType == InfoType::IT_default)
-OutString = Ref.UnresolvedName;
-  else {
-StringUSR = llvm::toHex(llvm::toStringRef(Ref.USR));
-OutString = StringUSR;
-  }
-  if (!prepRecordData(ID, !OutString.empty()))
-return;
-  assert(OutString.size() < (1U << BitCodeConstants::StringLengthSize));
-  Record.push_back((int)Ref.RefType);
-  Record.push_back(OutString.size());
-  Stream.EmitRecordWithBlob(Abbrevs.get(ID), Record, OutString);
-}
-
 void ClangDocBitcodeWriter::emitRecord(bool Val, RecordId ID) {
   assert(RecordIdNameMap[ID] && "Unknown RecordId.");
   assert(RecordIdNameMap[ID].Abbrev == &BoolAbbrev && "Abbrev type mismatch.");
@@ -408,28 +374,37 @@ void ClangDocBitcodeWriter::emitBlockInf
 
 // Block emission
 
+void ClangDocBitcodeWriter::emitBlock(const Reference &R, FieldId Field) {
+  if (R.USR == EmptySID && R.Name.empty())
+return;
+  StreamSubBlockGuard Block(Stream, BI_REFERENCE_BLOCK_ID);
+  emitRecord(R.USR, REFERENCE_USR);
+  emitRecord(R.Name, REFERENCE_NAME);
+  emitRecord((unsigned)R.RefType, REFERENCE_TYPE);
+  emitRecord((unsigned)Field, REFERENCE_FIELD);
+}
+
 void ClangDocBitcodeWriter::emitBlock(const TypeInfo &T) {
   StreamSubBlockGuard Block(Stream, BI_TYPE_BLOCK_ID);
-  emitRecord(T.Type, TYPE_REF);
+  emitBlock(T.Type, FieldId::F_type);
 }
 
 void ClangDocBitcodeWriter::emitBlock(const FieldTypeInfo &T) {
   StreamSubBlockGuard Block(Stream, BI_FIELD_TYPE_BLOCK_ID);
-  emitRecord(T.Type, FIELD_TYPE_REF);
+  emitBlock(T.Type, FieldId::F_type);
   emitRecord(T.Name, FIELD_TYPE_NAME);
 }
 
 void ClangDocBitcodeWriter::emitBlock(const MemberTypeInfo &T) {
   StreamSubBlockGuard Block(Stream, BI_MEMBER_TYPE_BLOCK_ID);
-  emitRecord(T.Type, MEMBER_TYPE_REF);
+  emitBlock(T.Type, FieldId::F_type);
   emitRecord(T.Name, MEMBER_TYPE_NAME);
   emitRecord(T.Access, MEMBER_TYPE_ACCESS);
 }
 
 void ClangDocBitcodeWriter::emitBlock(const CommentInfo &I) {
   StreamSubBlockGuard Block(Stream, BI_COMMENT_BLOCK_ID);
-  for (const auto &L :
-   std::vector>{
+  for (const auto &L : st

[PATCH] D40937: [clang-tidy] Infinite loop checker

[Alexander Kornienko via Phabricator via cfe-commits]

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

Could you run the check on llvm sources and post a summary of results here?

A few more inline comments.




Comment at: clang-tidy/bugprone/InfiniteLoopCheck.cpp:102
+  if (std::unique_ptr TheCFG =
+  CFG::buildCFG(nullptr, FunctionBody, &ASTCtx, Options))
+Sequence = llvm::make_unique(TheCFG.get(), &ASTCtx);

What does `nullptr` mean here? Please add an argument comment 
(`/*ArgumentName=*/nullptr, ...`).



Comment at: clang-tidy/bugprone/InfiniteLoopCheck.cpp:109-112
+  const auto *ContainingLambda =
+  Result.Nodes.getNodeAs("containing-lambda");
+  const auto *ContainingFunc =
+  Result.Nodes.getNodeAs("containing-func");

nit: Let's put these variable definitions into the corresponding `if` 
conditions to limit their scope. I'd also move the `if`s to the start of the 
function.



Comment at: clang-tidy/bugprone/InfiniteLoopCheck.cpp:122-123
+FunctionBody = ContainingFunc->getBody();
+  else
+return;
+

Instead I'd check that FunctionBody is not nullptr.



Comment at: clang-tidy/bugprone/InfiniteLoopCheck.cpp:148
+// (excluding the init stmt).
+if (const auto ForLoop = dyn_cast(LoopStmt)) {
+  if (ForLoop->getInc())

nit: `const auto *`



Comment at: clang-tidy/bugprone/InfiniteLoopCheck.cpp:150
+  if (ForLoop->getInc())
+Match = match(potentiallyModifyVarStmt(CondVar), *ForLoop->getInc(),
+  ASTCtx);

Any reason to store the result of the matching instead of returning early? Same 
below. Please also move the definition of the Match variable to where it's 
actually needed first time.



Comment at: clang-tidy/bugprone/InfiniteLoopCheck.cpp:175
+
+for (const auto &ES : Match) {
+  if (Sequence->potentiallyAfter(LoopStmt, ES.getNodeAs("escStmt")))

Please use the specific type instead of `auto`.



Comment at: clang-tidy/bugprone/InfiniteLoopCheck.h:37
+private:
+  bool updateSequence(Stmt *FunctionBody, ASTContext &ASTCtx);
+  const Stmt *PrevFunctionBody;

You seem to have forgotten to update the header.


https://reviews.llvm.org/D40937



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

[PATCH] D46281: [clang-doc] Attaching a name to reference data

[Julie Hockett via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
juliehockett marked 2 inline comments as done.
Closed by commit rL331539: [clang-doc] Attaching a name to reference data 
(authored by juliehockett, committed by ).
Herald added subscribers: llvm-commits, klimek.

Changed prior to commit:
  https://reviews.llvm.org/D46281?vs=144747&id=145218#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D46281

Files:
  clang-tools-extra/trunk/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/trunk/clang-doc/BitcodeWriter.h
  clang-tools-extra/trunk/clang-doc/Representation.h
  clang-tools-extra/trunk/clang-doc/Serialize.cpp
  clang-tools-extra/trunk/test/clang-doc/mapper-class-in-class.cpp
  clang-tools-extra/trunk/test/clang-doc/mapper-class-in-function.cpp
  clang-tools-extra/trunk/test/clang-doc/mapper-class.cpp
  clang-tools-extra/trunk/test/clang-doc/mapper-comments.cpp
  clang-tools-extra/trunk/test/clang-doc/mapper-enum.cpp
  clang-tools-extra/trunk/test/clang-doc/mapper-function.cpp
  clang-tools-extra/trunk/test/clang-doc/mapper-method.cpp
  clang-tools-extra/trunk/test/clang-doc/mapper-namespace.cpp
  clang-tools-extra/trunk/test/clang-doc/mapper-struct.cpp
  clang-tools-extra/trunk/test/clang-doc/mapper-union.cpp

Index: clang-tools-extra/trunk/clang-doc/BitcodeWriter.h
===
--- clang-tools-extra/trunk/clang-doc/BitcodeWriter.h
+++ clang-tools-extra/trunk/clang-doc/BitcodeWriter.h
@@ -31,7 +31,7 @@
 // Current version number of clang-doc bitcode.
 // Should be bumped when removing or changing BlockIds, RecordIds, or
 // BitCodeConstants, though they can be added without breaking it.
-static const unsigned VersionNumber = 1;
+static const unsigned VersionNumber = 2;
 
 struct BitCodeConstants {
   static constexpr unsigned RecordSize = 16U;
@@ -59,20 +59,20 @@
   BI_RECORD_BLOCK_ID,
   BI_FUNCTION_BLOCK_ID,
   BI_COMMENT_BLOCK_ID,
-  BI_FIRST = BI_VERSION_BLOCK_ID,
-  BI_LAST = BI_COMMENT_BLOCK_ID
+  BI_REFERENCE_BLOCK_ID,
+  BI_LAST,
+  BI_FIRST = BI_VERSION_BLOCK_ID
 };
 
 // New Ids need to be added to the enum here, and to the relevant IdNameMap and
 // initialization list in the implementation file.
-#define INFORECORDS(X) X##_USR, X##_NAME, X##_NAMESPACE
+#define INFORECORDS(X) X##_USR, X##_NAME
 
 enum RecordId {
   VERSION = 1,
   INFORECORDS(FUNCTION),
   FUNCTION_DEFLOCATION,
   FUNCTION_LOCATION,
-  FUNCTION_PARENT,
   FUNCTION_ACCESS,
   FUNCTION_IS_METHOD,
   COMMENT_KIND,
@@ -86,10 +86,7 @@
   COMMENT_ATTRKEY,
   COMMENT_ATTRVAL,
   COMMENT_ARG,
-  TYPE_REF,
-  FIELD_TYPE_REF,
   FIELD_TYPE_NAME,
-  MEMBER_TYPE_REF,
   MEMBER_TYPE_NAME,
   MEMBER_TYPE_ACCESS,
   INFORECORDS(NAMESPACE),
@@ -102,17 +99,22 @@
   RECORD_DEFLOCATION,
   RECORD_LOCATION,
   RECORD_TAG_TYPE,
-  RECORD_PARENT,
-  RECORD_VPARENT,
-  RI_FIRST = VERSION,
-  RI_LAST = RECORD_VPARENT
+  REFERENCE_USR,
+  REFERENCE_NAME,
+  REFERENCE_TYPE,
+  REFERENCE_FIELD,
+  RI_LAST,
+  RI_FIRST = VERSION
 };
 
-static constexpr unsigned BlockIdCount = BI_LAST - BI_FIRST + 1;
-static constexpr unsigned RecordIdCount = RI_LAST - RI_FIRST + 1;
+static constexpr unsigned BlockIdCount = BI_LAST - BI_FIRST;
+static constexpr unsigned RecordIdCount = RI_LAST - RI_FIRST;
 
 #undef INFORECORDS
 
+// Identifiers for differentiating between subblocks
+enum class FieldId { F_namespace = 1, F_parent, F_vparent, F_type };
+
 class ClangDocBitcodeWriter {
 public:
   ClangDocBitcodeWriter(llvm::BitstreamWriter &Stream) : Stream(Stream) {
@@ -137,6 +139,7 @@
   void emitBlock(const FieldTypeInfo &B);
   void emitBlock(const MemberTypeInfo &B);
   void emitBlock(const CommentInfo &B);
+  void emitBlock(const Reference &B, FieldId F);
 
 private:
   class AbbreviationMap {
Index: clang-tools-extra/trunk/clang-doc/Serialize.cpp
===
--- clang-tools-extra/trunk/clang-doc/Serialize.cpp
+++ clang-tools-extra/trunk/clang-doc/Serialize.cpp
@@ -173,21 +173,23 @@
 
 static void parseFields(RecordInfo &I, const RecordDecl *D) {
   for (const FieldDecl *F : D->fields()) {
-// FIXME: Set Access to the appropriate value.
-SymbolID Type;
-std::string Name;
-InfoType RefType;
 if (const auto *T = getDeclForType(F->getTypeSourceInfo()->getType())) {
-  Type = getUSRForDecl(T);
-  if (dyn_cast(T))
-RefType = InfoType::IT_enum;
-  else if (dyn_cast(T))
-RefType = InfoType::IT_record;
-  I.Members.emplace_back(Type, RefType, F->getQualifiedNameAsString());
-} else {
-  Name = F->getTypeSourceInfo()->getType().getAsString();
-  I.Members.emplace_back(Name, F->getQualifiedNameAsString());
+  // Use getAccessUnsafe so that we just get the default AS_none if it's not
+  // valid, as opposed to an assert.
+  if (const auto *N = dyn_cast(T)) {
+I.Members.emplace_back(getUSRForDecl(T), N->getNameAsString(),
+   InfoType::

Re: [PATCH] D36610: [Tooling] Add option to getFullyQualifiedName using a custom PritingPolicy

[Sterling Augustine via cfe-commits]

I applied this to a clean local copy, which has no other changes, and have
the following test error, which may be pilot error on my part, but
nevertheless, this test needs to be robust to changes in the line number.

llvm-svn/llvm/tools/clang/unittests/Tooling/QualTypeNamesTest.cpp:39:
Failure
Value of: false
  Actual: false
Expected: true
Typename::getFullyQualifiedName failed for (anonymous struct)::un_in_st_1
   Actual: union (anonymous struct at input.cc:1:1)::(anonymous union at
input.cc:2:27)
 Exepcted: union (anonymous struct at input.cc:1:1)::(anonymous union at
input.cc:2:31)



On Fri, May 4, 2018 at 9:22 AM, Mikhail Ramalho via Phabricator <
revi...@reviews.llvm.org> wrote:

> mikhail.ramalho added a comment.
>
> Thanks.
>
> I just need someone to push it, as I don't have write access to the repo.
>
>
> https://reviews.llvm.org/D36610
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46439: Fix incorrect packed aligned structure layout

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Test?


Repository:
  rC Clang

https://reviews.llvm.org/D46439



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

[PATCH] D46394: [clang-cl] Print /showIncludes to stderr, if used in combination with /E, /EP or /P

[Erich Keane via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC331533: [clang-cl] Print /showIncludes to stderr, if used in 
combination with /E, /EP… (authored by erichkeane, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D46394

Files:
  include/clang/Frontend/DependencyOutputOptions.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInstance.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/HeaderIncludeGen.cpp
  test/Driver/cl-options.c
  test/Frontend/print-header-includes.c

Index: lib/Frontend/HeaderIncludeGen.cpp
===
--- lib/Frontend/HeaderIncludeGen.cpp
+++ lib/Frontend/HeaderIncludeGen.cpp
@@ -80,9 +80,23 @@
const DependencyOutputOptions &DepOpts,
bool ShowAllHeaders, StringRef OutputPath,
bool ShowDepth, bool MSStyle) {
-  raw_ostream *OutputFile = MSStyle ? &llvm::outs() : &llvm::errs();
+  raw_ostream *OutputFile = &llvm::errs();
   bool OwnsOutputFile = false;
 
+  // Choose output stream, when printing in cl.exe /showIncludes style.
+  if (MSStyle) {
+switch (DepOpts.ShowIncludesDest) {
+default:
+  llvm_unreachable("Invalid destination for /showIncludes output!");
+case ShowIncludesDestination::Stderr:
+  OutputFile = &llvm::errs();
+  break;
+case ShowIncludesDestination::Stdout:
+  OutputFile = &llvm::outs();
+  break;
+}
+  }
+
   // Open the output file, if used.
   if (!OutputPath.empty()) {
 std::error_code EC;
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1130,7 +1130,17 @@
   Opts.ShowHeaderIncludes = Args.hasArg(OPT_H);
   Opts.HeaderIncludeOutputFile = Args.getLastArgValue(OPT_header_include_file);
   Opts.AddMissingHeaderDeps = Args.hasArg(OPT_MG);
-  Opts.PrintShowIncludes = Args.hasArg(OPT_show_includes);
+  if (Args.hasArg(OPT_show_includes)) {
+// Writing both /showIncludes and preprocessor output to stdout
+// would produce interleaved output, so use stderr for /showIncludes.
+// This behaves the same as cl.exe, when /E, /EP or /P are passed.
+if (Args.hasArg(options::OPT_E) || Args.hasArg(options::OPT_P))
+  Opts.ShowIncludesDest = ShowIncludesDestination::Stderr;
+else
+  Opts.ShowIncludesDest = ShowIncludesDestination::Stdout;
+  } else {
+Opts.ShowIncludesDest = ShowIncludesDestination::None;
+  }
   Opts.DOTOutputFile = Args.getLastArgValue(OPT_dependency_dot);
   Opts.ModuleDependencyOutputDir =
   Args.getLastArgValue(OPT_module_dependency_dir);
Index: lib/Frontend/CompilerInstance.cpp
===
--- lib/Frontend/CompilerInstance.cpp
+++ lib/Frontend/CompilerInstance.cpp
@@ -462,7 +462,7 @@
/*ShowDepth=*/false);
   }
 
-  if (DepOpts.PrintShowIncludes) {
+  if (DepOpts.ShowIncludesDest != ShowIncludesDestination::None) {
 AttachHeaderIncludeGen(*PP, DepOpts,
/*ShowAllHeaders=*/true, /*OutputPath=*/"",
/*ShowDepth=*/true, /*MSStyle=*/true);
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -5082,13 +5082,8 @@
 CmdArgs.push_back("--dependent-lib=oldnames");
   }
 
-  // Both /showIncludes and /E (and /EP) write to stdout. Allowing both
-  // would produce interleaved output, so ignore /showIncludes in such cases.
-  if ((!Args.hasArg(options::OPT_E) && !Args.hasArg(options::OPT__SLASH_EP)) ||
-  (Args.hasArg(options::OPT__SLASH_P) &&
-   Args.hasArg(options::OPT__SLASH_EP) && !Args.hasArg(options::OPT_E)))
-if (Arg *A = Args.getLastArg(options::OPT_show_includes))
-  A->render(Args, CmdArgs);
+  if (Arg *A = Args.getLastArg(options::OPT_show_includes))
+A->render(Args, CmdArgs);
 
   // This controls whether or not we emit RTTI data for polymorphic types.
   if (Args.hasFlag(options::OPT__SLASH_GR_, options::OPT__SLASH_GR,
Index: include/clang/Frontend/DependencyOutputOptions.h
===
--- include/clang/Frontend/DependencyOutputOptions.h
+++ include/clang/Frontend/DependencyOutputOptions.h
@@ -15,6 +15,9 @@
 
 namespace clang {
 
+/// ShowIncludesDestination - Destination for /showIncludes output.
+enum class ShowIncludesDestination { None, Stdout, Stderr };
+
 /// DependencyOutputFormat - Format for the compiler dependency file.
 enum class DependencyOutputFormat { Make, NMake };
 
@@ -28,9 +31,11 @@
  /// dependency, which can avoid some 'make'
  /// problems.
   unsigned AddMissingHeade

[PATCH] D44435: CUDA ctor/dtor Module-Unique Symbol Name

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

Perhaps we should take a step back and consider whether this is the right 
approach to solve your problem.

If I understand it correctly, the real issue is that you repeatedly recompile 
the same module and cling will only use the function from the first module it's 
seen it in. Unlike regular functions that presumably remain the same in all the 
modules they are present in, CUDA constructors do change and you need cling to 
grab the one from the most recent module.

This patch deals with the issue by attempting to add a unique sufix. Presumably 
cling will then generate some sort of unique module name and will get unique 
constructor name in return. The down side of this approach is that module name 
is something that is derived from the file name and the functionality you're 
changing is in the shared code, so you need to make sure that whatever you 
implement makes sense for LLVM in general and that it does what it claims it 
does. AFAICT, LLVM has no pressing need for the unique constructor name -- it's 
a function with internal linkage and, if we ever need to generate more than 
one, LLVM is capable of generating unique names within the module all by 
itself. The patch currently does not fulfill the "unique" part either.

Perhaps you should consider a different approach which could handle the issue 
completely in cling. E.g. You could rename the constructor in the module's IR 
before passing it to JIT. Or you could rename it in PTX (it's just text after 
all) before passing it to driver or PTXAS.




Comment at: lib/CodeGen/CGCUDANV.cpp:287
+CtorSuffix.append("_");
+CtorSuffix.append(ModuleName);
+  }

SimeonEhrig wrote:
> tra wrote:
> > SimeonEhrig wrote:
> > > tra wrote:
> > > > There is a general problem with this approach. File name can contain 
> > > > the characters that PTX does not allow.
> > > > We currently only deal with '.' and '@', but that's not enough here.
> > > > You may want to either mangle the name somehow to avoid/convert illegal 
> > > > characters or use some other way to provide unique suffix. Hex-encoded 
> > > > hash of the file name would avoid this problem, for example.
> > > > 
> > > > 
> > > > 
> > > Maybe I'm wrong but I think, that should be no problem, because the 
> > > generating of a cuda ctor/dtor have nothing to do with the PTX 
> > > generation. 
> > > 
> > > The function 'makeModuleCtorFunction' should just generate llvm ir code 
> > > for the host (e.g. x86_64).
> > > 
> > > If I'm wrong, could you tell me please, where in the source code the 
> > > 'makeModuleCtorFunction' affect the PTX generation.
> > You are correct that PTX is irrelevant here. I've completely missed that 
> > this will be generated for the host, which is more forgiving. 
> > 
> > That said, I'm still not completely sure whether we're guaranteed that 
> > using arbitrary characters in a symbol name is OK on x86 and, potentially, 
> > other host platforms. As an experiment, try using a module which has a 
> > space in its name.
> At line 295 and 380 in CGCUDANV.cpp I use a sanitizer function, which replace 
> all symbols without [a-zA-Z0-9._] with a '_'. It's the same solution like in 
> D34059. So I think, it would works in general.
> 
> Only for information. I tested it with a module name, which includes a 
> whitespace and without the sanitizer. It works on Linux x86 and the ELF 
> format. There was an whitespace in the symbol of the cuda module ctor (I 
> checked it with readelf).
> 
> In general, do you think my solution approach is technically okay? Your 
> answer will be really helpful for internal usage in our cling project. At the 
> moment I developed the cling-cuda-interpreter based on this patch and it 
> would helps a lot of, if I can say, that the patch doesn't cause any problem 
> with the CUDA-environment.  
This still does not give you unique suffix which was stated at the main goal of 
this patch. E.g files "foo$bar" and "foo_bar" will produce identical names. See 
my previous comment regarding using a hash.







https://reviews.llvm.org/D44435



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

[PATCH] D45944: Disallow pointers to const in __sync_fetch_and_xxx

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

LGTM, although you might consider testing a broader set of builtins.  Maybe at 
least one from the `__atomic_*` family?


https://reviews.llvm.org/D45944



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

[PATCH] D46441: [clang][CodeGenCXX] Noalias attr for copy/move constructor arguments

[Anton Bikineev via Phabricator via cfe-commits]

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

The patch addresses this bug .

According to the Standard (taken from the bug description):
[class.ctor] paragraph 14:

"During the construction of an object, if the value of the object or any of its 
subobjects is accessed through a glvalue that is not obtained, directly or 
indirectly, from the constructor’s this pointer, the value of the object or 
subobject thus obtained is unspecified."


Repository:
  rC Clang

https://reviews.llvm.org/D46441

Files:
  lib/CodeGen/CGCXX.cpp


Index: lib/CodeGen/CGCXX.cpp
===
--- lib/CodeGen/CGCXX.cpp
+++ lib/CodeGen/CGCXX.cpp
@@ -224,6 +224,11 @@
   } else {
 const auto *CD = cast(MD);
 GD = GlobalDecl(CD, toCXXCtorType(Type));
+
+// As long as `this` argument always comes first
+// we hardcode the position of the `src` argument
+if (CD->isCopyOrMoveConstructor())
+  Fn->addParamAttr(1, llvm::Attribute::NoAlias);
   }
 
   setFunctionLinkage(GD, Fn);


Index: lib/CodeGen/CGCXX.cpp
===
--- lib/CodeGen/CGCXX.cpp
+++ lib/CodeGen/CGCXX.cpp
@@ -224,6 +224,11 @@
   } else {
 const auto *CD = cast(MD);
 GD = GlobalDecl(CD, toCXXCtorType(Type));
+
+// As long as `this` argument always comes first
+// we hardcode the position of the `src` argument
+if (CD->isCopyOrMoveConstructor())
+  Fn->addParamAttr(1, llvm::Attribute::NoAlias);
   }
 
   setFunctionLinkage(GD, Fn);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46042: Cap vector alignment at 16 for all Darwin platforms

[Ahmed Bougacha via Phabricator via cfe-commits]

ab added a comment.

So, this makes sense to me, but on x86, should we also be worried about the 
fact that the calling convention is based on which features are available?  
(>128bit ext_vector_types are passed in AVX/AVX-512 registers, if available).  
Presumably swift is also affected, no?


Repository:
  rC Clang

https://reviews.llvm.org/D46042



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

[PATCH] D46441: [clang][CodeGenCXX] Noalias attr for copy/move constructor arguments

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

Makes sense to me.


Repository:
  rC Clang

https://reviews.llvm.org/D46441



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

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D45476#1087487, @EricWF wrote:

> In https://reviews.llvm.org/D45476#1087446, @cfe-commits wrote:
>
> > I think you and Richard agreed that you weren’t going to synthesize a whole
> >  expression tree at every use of the operator, and I agree with that
> >  decision. That’s very different from what I’m asking you to do, which is to
> >  synthesize in isolation a call to the copy-constructor.
>
>
> Perhaps. My apologies. I'm still quite new to the Clang internals. I 
> appreciate your patience.
>
> > There are several places in the compiler that require these implicit copies 
> > which aren’t just
> >  normal expressions; this is the common pattern for handling them. The
> >  synthesized expression can be emitted multiple times, and it can be freely
> >  re-synthesized in different translation units instead of being serialized.
>
> I'm not sure this is always the case. For example:
>
>   // foo.h -- compiled as module.
>   #include 
>   struct Foo { int x; };
>   inline auto operator<=>(Foo const& LHS, Foo const& RHS) {
> // CXXConstructExpr's would be built when initially building the 
> expression
> // below. But the caches in ASTContext would not be serialized.
> return LHS.x <=> RHS.x;
>   }
>   // foo.cpp
>   #include  // imported via module.
>   auto bar(Foo LHS, Foo RHS) {
> // The expression below calls a user defined comparison operator,
> // so Sema doesn't process any of the types in , but it
> // does generate code for the inline function, which requires ;
> // but it's too late to synthesize a CXXConstructExpr. 
> return LHS <=> RHS;
>   }
>
>
>
>
> > You’re already finding and caching a constructor; storing a
> >  CXXConstructExpr is basically thr same thing, but in a nicer form that
> >  handles more cases and doesn’t require as much redundant code in IRGen.
>
> I'm not actually caching the copy constructor. And I disagree that storing a
>  `CXXConstructExpr` is essentially the same thing. I can lookup the 
> `CXXConstructorDecl` without `Sema`,
>  but I can't build a `CXXConstructExpr` without it.


Ah.  If you want to be able to find this thing without a Sema around in order to
handle deserialized expressions by just caching things in the ASTContext, yes,
I agree that it would be difficult to build a `CXXConstructExpr` correctly.  I 
don't
fully understand the goal of avoiding serialization here, though.

>> STLs *frequently* make use of default arguments on copy constructors (for
>>  allocators). I agree that it’s unlikely that that would happen here, but
>>  that’s precisely because it’s unlikely that this type would ever be
>>  non-trivial.
> 
> A couple of points. First, when I say copy constructor, I mean the special 
> member, which
>  cannot have default arguments,

I'm sorry, but this is just not true.  The formation rules for a copy 
constructor are laid
out in [class.copy]p2, and they explicitly allow default arguments.

> Also note that it's always the case that at least one copy constructor 
> participates
>  in overload resolution.

But it's not true that that copy constructor has to be selected by overload 
resolution.

> Second, in the synopsis for the STL types, no constructors are declared. 
> Although I don't
>  think the standard spells it out anywhere, I believe this forbids the types 
> from having user
>  defined constructors (though perhaps not user-declared explicitly defaulted 
> constructors.
>  For example adding a user defined destructor would be non-conforming since 
> it makes
>  the types non-literal).

That would certainly be helpful, but I don't think it's true; in general the 
standard describes
what things are guaranteed to work with library types, but it doesn't generally 
constrain
the existence of other operations.

> Third, the STL spec implicitly requires the comparison category types be 
> `CopyConstructible`
>  (The comparison operators are pass by value, and the valid values are 
> declared as const).

Yes, of course.

> Barring STL maintainers that are out of their mind, I posit that the copy 
> constructor
>  `T(T const&)` will always be available.  However, the STL types are free to 
> add base
>  classes and fields arbitrarily. I could imagine some weird reasons why STL's 
> might
>  want to have non-trivial members or bases.

I think it is reasonable to assume that these types will always be 
copy-constructible
from a const l-value, but as mentioned above, that doesn't mean the 
copy-constructor
has to be declared as `T(T const &)`.

>> Mostly, though, I don’t understand the point of imposing a partial set of
>>  non-conformant restrictions on the type. It’s easy to support an arbitrary
>>  copy constructor by synthesizing a CXXConstructExpr, and this will
>>  magically take care of any constexpr issues, as well as removing the need
>>  for open-coding a constructor call.
> 
> Fully checking the validity of copy construction would be preferable. I'll
>  atte

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

[Paul Fultz II via Phabricator via cfe-commits]

pfultz2 added a comment.

There doesn't seem to be a simple way to remove it from the ClangTidyOptions 
class, as a lot more functions need to be changed to support that. I would 
prefer to leave it there for now, and we can refactor it later. Either way, the 
check can't be set from a config file.


https://reviews.llvm.org/D46159



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

[PATCH] D46042: Cap vector alignment at 16 for all Darwin platforms

[Steve Canon via Phabricator via cfe-commits]

scanon added a comment.

In https://reviews.llvm.org/D46042#1088044, @ab wrote:

> So, this makes sense to me, but on x86, should we also be worried about the 
> fact that the calling convention is based on which features are available?  
> (>128bit ext_vector_types are passed in AVX/AVX-512 registers, if available). 
>  Presumably swift is also affected, no?


Swift's calling conventions (will?) always divide larger vectors into 128b 
pieces. When interacting with C conventions, yes, this is still an issue.


Repository:
  rC Clang

https://reviews.llvm.org/D46042



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

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

In https://reviews.llvm.org/D46159#1086732, @aaron.ballman wrote:

> If the static analyzer people desire this feature, that would sway my 
> position on it, but it sounds like they're hesitant as well. 
>  However, I don't think clang-tidy is beholden either -- if we don't think 
> this functionality should be exposed and can justify that position, that 
> should carry weight as well. \
>  From a policy perspective, I would be fine with a policy for clang-tidy 
> where we never expose an alpha checker from the static analyzer (or only 
> expose checks on a case by case basis) because I don't mind users having to 
> jump through hoops to get to experimental, unsupported functionality.


As folks noted here, some users prefer to use clang-tidy as a frontend for the 
static analyzer. If this helps them test experimental CSA features and CSA 
maintainers are willing to accept bug reports and potentially patches from this 
category of users, I don't want to create obstacles, as long as all 
experimental features need to be explicitly enabled.

Devin, what's your take on this?

> I primarily don't like the fact that, as a user, I enable checks by name but 
> for some kinds of checks I have to *also* enable them via a secondary 
> mechanism otherwise the name doesn't even exist. This strikes me as being a 
> likely source of confusion where forgetting one flag causes behavioral 
> differences the user doesn't expect.

This particular aspect doesn't seem problematic to me. In order to observe 
these behavioral differences, the user would have to specify this flag at least 
once, get the results of experimental checkers and see the disclaimer, if we 
decide to add one (see below). I guess, forgetting about the existence of this 
flag would be quite unlikely. Forgetting the spelling of the flag should not 
cause any confusion - it can be looked up in the source code, found out using 
`clang-tidy -help-hidden` or asked about.

>>> Making the flag sound scary doesn't suffice -- many users never see the 
>>> flags because they're hidden away in a build script, but they definitely 
>>> see the diagnostics and file bug reports.

As an additional way to ensure that this flag doesn't get specified by mistake 
/ doesn't get buried and forgotten inside a script, clang-tidy can print a 
disclaimer each time it's executed with this flag.




Comment at: clang-tidy/ClangTidyOptions.h:80-82
+  /// \brief Turns on experimental alpha checkers from the static analyzer.
+  llvm::Optional AllowEnablingAlphaChecks;
+

pfultz2 wrote:
> alexfh wrote:
> > Since this will only be configurable via a flag, this option will be global 
> > (i.e. not depend on the location of the file being analyzed). I'd suggest 
> > to remove this option altogether and use something else to pass it to 
> > ClangTidyASTConsumerFactory. It could be stashed into 
> > ClangTidyASTConsumerFactory and passed as a parameter of runClangTidy,  or 
> > it could live in ClangTidyContext.
> But it needs to be passed to `getCheckNames` as well, which doesn't take a 
> `ClangTidyContext`.
We can add a boolean parameter to `getCheckNames`. 
`ClangTidyASTConsumerFactory` could get this as a constructor parameter or from 
`ClangTidyContext`.


https://reviews.llvm.org/D46159



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

[PATCH] D46441: [clang][CodeGenCXX] Noalias attr for copy/move constructor arguments

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Please add a test.


Repository:
  rC Clang

https://reviews.llvm.org/D46441



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

[PATCH] D46441: [clang][CodeGenCXX] Noalias attr for copy/move constructor arguments

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Test cases?


Repository:
  rC Clang

https://reviews.llvm.org/D46441



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

r331544 - [clang-format] Refactor #include insertion/deletion functionality into a class.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Fri May  4 10:55:13 2018
New Revision: 331544

URL: http://llvm.org/viewvc/llvm-project?rev=331544&view=rev
Log:
[clang-format] Refactor #include insertion/deletion functionality into a class.

Summary:
The class will be moved into libToolingCore as followup.

The new behaviors in this patch:
- New #include is inserted in the right position in a #include block to
preserver sorted #includes. This is best effort - only works when the
block is already sorted.
- When inserting multiple #includes to the end of a file which doesn't
end with a "\n" character, a "\n" will be prepended to each #include.
This is a special and rare case that was previously handled. This is now
relaxed to avoid complexity as it's rare in practice.

Reviewers: ilya-biryukov

Reviewed By: ilya-biryukov

Subscribers: klimek, cfe-commits, djasper

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

Modified:
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/unittests/Format/CleanupTest.cpp

Modified: cfe/trunk/lib/Format/Format.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=331544&r1=331543&r2=331544&view=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Fri May  4 10:55:13 2018
@@ -40,7 +40,9 @@
 #include "llvm/Support/YAMLTraits.h"
 #include 
 #include 
+#include 
 #include 
+#include 
 
 #define DEBUG_TYPE "format-formatter"
 
@@ -1706,7 +1708,8 @@ public:
   // Returns the priority of the category which \p IncludeName belongs to.
   // If \p CheckMainHeader is true and \p IncludeName is a main header, returns
   // 0. Otherwise, returns the priority of the matching category or INT_MAX.
-  int getIncludePriority(StringRef IncludeName, bool CheckMainHeader) {
+  // NOTE: this API is not thread-safe!
+  int getIncludePriority(StringRef IncludeName, bool CheckMainHeader) const {
 int Ret = INT_MAX;
 for (unsigned i = 0, e = CategoryRegexs.size(); i != e; ++i)
   if (CategoryRegexs[i].match(IncludeName)) {
@@ -1739,7 +1742,8 @@ private:
   bool IsMainFile;
   StringRef FileName;
   StringRef FileStem;
-  SmallVector CategoryRegexs;
+  // Regex is not thread-safe.
+  mutable SmallVector CategoryRegexs;
 };
 
 const char IncludeRegexPattern[] =
@@ -2003,10 +2007,228 @@ unsigned getMaxHeaderInsertionOffset(Str
   });
 }
 
-bool isDeletedHeader(llvm::StringRef HeaderName,
- const std::set &HeadersToDelete) {
-  return HeadersToDelete.count(HeaderName) ||
- HeadersToDelete.count(HeaderName.trim("\"<>"));
+/// Generates replacements for inserting or deleting #include directives in a
+/// file.
+class HeaderIncludes {
+public:
+  HeaderIncludes(llvm::StringRef FileName, llvm::StringRef Code,
+ const FormatStyle &Style);
+
+  /// Inserts an #include directive of \p Header into the code. If \p IsAngled
+  /// is true, \p Header will be quoted with <> in the directive; otherwise, it
+  /// will be quoted with "".
+  ///
+  /// When searching for points to insert new header, this ignores #include's
+  /// after the #include block(s) in the beginning of a file to avoid inserting
+  /// headers into code sections where new #include's should not be added by
+  /// default. These code sections include:
+  ///   - raw string literals (containing #include).
+  ///   - #if blocks.
+  ///   - Special #include's among declarations (e.g. functions).
+  ///
+  /// Returns a replacement that inserts the new header into a suitable 
#include
+  /// block of the same category. This respects the order of the existing
+  /// #includes in the block; if the existing #includes are not already sorted,
+  /// this will simply insert the #include in front of the first #include of 
the
+  /// same category in the code that should be sorted after \p IncludeName. If
+  /// \p IncludeName already exists (with exactly the same spelling), this
+  /// returns None.
+  llvm::Optional insert(llvm::StringRef Header,
+  bool IsAngled) const;
+
+  /// Removes all existing #includes of \p Header quoted with <> if \p IsAngled
+  /// is true or "" if \p IsAngled is false.
+  /// This doesn't resolve the header file path; it only deletes #includes with
+  /// exactly the same spelling.
+  tooling::Replacements remove(llvm::StringRef Header, bool IsAngled) const;
+
+private:
+  struct Include {
+Include(StringRef Name, tooling::Range R) : Name(Name), R(R) {}
+
+// An include header quoted with either <> or "".
+std::string Name;
+// The range of the whole line of include directive including any eading
+// whitespaces and trailing comment.
+tooling::Range R;
+  };
+
+  void addExistingInclude(Include IncludeToAdd, unsigned NextLineOffset);
+
+  std::string FileName;
+  std::string Code;
+
+  // Map from include name (quotation trimmed) to a list of existing includes
+  // (in case there are 

[PATCH] D46180: [clang-format] Refactor #include insertion/deletion functionality into a class.

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC331544: [clang-format] Refactor #include insertion/deletion 
functionality into a class. (authored by ioeric, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D46180?vs=145210&id=145225#toc

Repository:
  rC Clang

https://reviews.llvm.org/D46180

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

Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -40,7 +40,9 @@
 #include "llvm/Support/YAMLTraits.h"
 #include 
 #include 
+#include 
 #include 
+#include 
 
 #define DEBUG_TYPE "format-formatter"
 
@@ -1706,7 +1708,8 @@
   // Returns the priority of the category which \p IncludeName belongs to.
   // If \p CheckMainHeader is true and \p IncludeName is a main header, returns
   // 0. Otherwise, returns the priority of the matching category or INT_MAX.
-  int getIncludePriority(StringRef IncludeName, bool CheckMainHeader) {
+  // NOTE: this API is not thread-safe!
+  int getIncludePriority(StringRef IncludeName, bool CheckMainHeader) const {
 int Ret = INT_MAX;
 for (unsigned i = 0, e = CategoryRegexs.size(); i != e; ++i)
   if (CategoryRegexs[i].match(IncludeName)) {
@@ -1739,7 +1742,8 @@
   bool IsMainFile;
   StringRef FileName;
   StringRef FileStem;
-  SmallVector CategoryRegexs;
+  // Regex is not thread-safe.
+  mutable SmallVector CategoryRegexs;
 };
 
 const char IncludeRegexPattern[] =
@@ -2003,10 +2007,228 @@
   });
 }
 
-bool isDeletedHeader(llvm::StringRef HeaderName,
- const std::set &HeadersToDelete) {
-  return HeadersToDelete.count(HeaderName) ||
- HeadersToDelete.count(HeaderName.trim("\"<>"));
+/// Generates replacements for inserting or deleting #include directives in a
+/// file.
+class HeaderIncludes {
+public:
+  HeaderIncludes(llvm::StringRef FileName, llvm::StringRef Code,
+ const FormatStyle &Style);
+
+  /// Inserts an #include directive of \p Header into the code. If \p IsAngled
+  /// is true, \p Header will be quoted with <> in the directive; otherwise, it
+  /// will be quoted with "".
+  ///
+  /// When searching for points to insert new header, this ignores #include's
+  /// after the #include block(s) in the beginning of a file to avoid inserting
+  /// headers into code sections where new #include's should not be added by
+  /// default. These code sections include:
+  ///   - raw string literals (containing #include).
+  ///   - #if blocks.
+  ///   - Special #include's among declarations (e.g. functions).
+  ///
+  /// Returns a replacement that inserts the new header into a suitable #include
+  /// block of the same category. This respects the order of the existing
+  /// #includes in the block; if the existing #includes are not already sorted,
+  /// this will simply insert the #include in front of the first #include of the
+  /// same category in the code that should be sorted after \p IncludeName. If
+  /// \p IncludeName already exists (with exactly the same spelling), this
+  /// returns None.
+  llvm::Optional insert(llvm::StringRef Header,
+  bool IsAngled) const;
+
+  /// Removes all existing #includes of \p Header quoted with <> if \p IsAngled
+  /// is true or "" if \p IsAngled is false.
+  /// This doesn't resolve the header file path; it only deletes #includes with
+  /// exactly the same spelling.
+  tooling::Replacements remove(llvm::StringRef Header, bool IsAngled) const;
+
+private:
+  struct Include {
+Include(StringRef Name, tooling::Range R) : Name(Name), R(R) {}
+
+// An include header quoted with either <> or "".
+std::string Name;
+// The range of the whole line of include directive including any eading
+// whitespaces and trailing comment.
+tooling::Range R;
+  };
+
+  void addExistingInclude(Include IncludeToAdd, unsigned NextLineOffset);
+
+  std::string FileName;
+  std::string Code;
+
+  // Map from include name (quotation trimmed) to a list of existing includes
+  // (in case there are more than one) with the name in the current file. 
+  // and "x" will be treated as the same header when deleting #includes.
+  llvm::StringMap> ExistingIncludes;
+
+  /// Map from priorities of #include categories to all #includes in the same
+  /// category. This is used to find #includes of the same category when
+  /// inserting new #includes. #includes in the same categories are sorted in
+  /// in the order they appear in the source file.
+  /// See comment for "FormatStyle::IncludeCategories" for details about include
+  /// priorities.
+  std::unordered_map>
+  IncludesByPriority;
+
+  int FirstIncludeOffset;
+  // All new headers should be inserted after this offset (e.g. after header
+  // guards, file comment).
+  unsigned MinInsertOffset;
+  // Max insertion offset in the original code. For example

[PATCH] D46042: Cap vector alignment at 16 for all Darwin platforms

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D46042#1088044, @ab wrote:

> So, this makes sense to me, but on x86, should we also be worried about the 
> fact that the calling convention is based on which features are available?  
> (>128bit ext_vector_types are passed in AVX/AVX-512 registers, if available). 
>  Presumably swift is also affected, no?


I'd forgotten about that.  I think there's a strong argument that we're 
required to pass at least the Intel intrinsic vector types that way, yeah.  But 
if we want a stable ABI for other vector types, we really can't.  The root 
problem here is that the Intel ABI seems to imagine that these vector types 
only exist when they're supported directly by hardware.  (And the Intel 
intrinsic headers do define those types even when AVX is disabled!)  So I don't 
know that we can make a good ABI story for that.


Repository:
  rC Clang

https://reviews.llvm.org/D46042



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

[PATCH] D46042: Cap vector alignment at 16 for all Darwin platforms

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D46042#1088049, @scanon wrote:

> In https://reviews.llvm.org/D46042#1088044, @ab wrote:
>
> > So, this makes sense to me, but on x86, should we also be worried about the 
> > fact that the calling convention is based on which features are available?  
> > (>128bit ext_vector_types are passed in AVX/AVX-512 registers, if 
> > available).  Presumably swift is also affected, no?
>
>
> Swift's calling conventions (will?) always divide larger vectors into 128b 
> pieces. When interacting with C conventions, yes, this is still an issue.


Right, this is just a C ABI issue.


Repository:
  rC Clang

https://reviews.llvm.org/D46042



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

[PATCH] D46374: Add support for ObjC property name to be a single acronym.

[Yan Zhang via Phabricator via cfe-commits]

Wizard updated this revision to Diff 145226.
Wizard added a comment.

optimize matching


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46374

Files:
  clang-tidy/objc/PropertyDeclarationCheck.cpp
  test/clang-tidy/objc-property-declaration-custom.m
  test/clang-tidy/objc-property-declaration.m


Index: test/clang-tidy/objc-property-declaration.m
===
--- test/clang-tidy/objc-property-declaration.m
+++ test/clang-tidy/objc-property-declaration.m
@@ -21,6 +21,7 @@
 @property(assign, nonatomic) int enable2GBackgroundFetch;
 @property(assign, nonatomic) int shouldUseCFPreferences;
 @property(assign, nonatomic) int enableGLAcceleration;
+@property(assign, nonatomic) int ID;
 @end
 
 @interface Foo (Bar)
Index: test/clang-tidy/objc-property-declaration-custom.m
===
--- test/clang-tidy/objc-property-declaration-custom.m
+++ test/clang-tidy/objc-property-declaration-custom.m
@@ -14,4 +14,5 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'ABC_custom_prefix' 
not using lowerCamelCase style or not prefixed in a category, according to the 
Apple Coding Guidelines [objc-property-declaration]
 @property(assign, nonatomic) int GIFIgnoreStandardAcronym;
 // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 
'GIFIgnoreStandardAcronym' not using lowerCamelCase style or not prefixed in a 
category, according to the Apple Coding Guidelines [objc-property-declaration]
+@property(strong, nonatomic) NSString *TGIF;
 @end
Index: clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -217,6 +217,12 @@
   assert(MatchedDecl->getName().size() > 0);
   auto *DeclContext = MatchedDecl->getDeclContext();
   auto *CategoryDecl = llvm::dyn_cast(DeclContext);
+
+  auto AcronymsRegex =
+  llvm::Regex("^" + AcronymsGroupRegex(EscapedAcronyms) + "$");
+  if (AcronymsRegex.match(MatchedDecl->getName())) {
+return;
+  }
   if (CategoryDecl != nullptr &&
   hasCategoryPropertyPrefix(MatchedDecl->getName())) {
 if (!prefixedPropertyNameValid(MatchedDecl->getName(), EscapedAcronyms) ||


Index: test/clang-tidy/objc-property-declaration.m
===
--- test/clang-tidy/objc-property-declaration.m
+++ test/clang-tidy/objc-property-declaration.m
@@ -21,6 +21,7 @@
 @property(assign, nonatomic) int enable2GBackgroundFetch;
 @property(assign, nonatomic) int shouldUseCFPreferences;
 @property(assign, nonatomic) int enableGLAcceleration;
+@property(assign, nonatomic) int ID;
 @end
 
 @interface Foo (Bar)
Index: test/clang-tidy/objc-property-declaration-custom.m
===
--- test/clang-tidy/objc-property-declaration-custom.m
+++ test/clang-tidy/objc-property-declaration-custom.m
@@ -14,4 +14,5 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'ABC_custom_prefix' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
 @property(assign, nonatomic) int GIFIgnoreStandardAcronym;
 // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'GIFIgnoreStandardAcronym' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
+@property(strong, nonatomic) NSString *TGIF;
 @end
Index: clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -217,6 +217,12 @@
   assert(MatchedDecl->getName().size() > 0);
   auto *DeclContext = MatchedDecl->getDeclContext();
   auto *CategoryDecl = llvm::dyn_cast(DeclContext);
+
+  auto AcronymsRegex =
+  llvm::Regex("^" + AcronymsGroupRegex(EscapedAcronyms) + "$");
+  if (AcronymsRegex.match(MatchedDecl->getName())) {
+return;
+  }
   if (CategoryDecl != nullptr &&
   hasCategoryPropertyPrefix(MatchedDecl->getName())) {
 if (!prefixedPropertyNameValid(MatchedDecl->getName(), EscapedAcronyms) ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46374: Add support for ObjC property name to be a single acronym.

[Yan Zhang via Phabricator via cfe-commits]

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



Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:222
+  [MatchedDecl](std::string const &s) {
+auto Acronym  = llvm::Regex("^" + s + "$");
+return Acronym.match(MatchedDecl->getName());

benhamilton wrote:
> Please be aware this will change the match from running a single regular 
> expression to running ~ 70 regular expressions on every single `@property`. I 
> would expect this to perform pretty poorly.
> 
This is a good point. If using matching instead of equality check, I should not 
use any_of then. Updated it to use AcronymsGroupRegex().


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46374



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

[PATCH] D46374: Add support for ObjC property name to be a single acronym.

[Yan Zhang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Wizard marked an inline comment as done.
Closed by commit rL331545: Add support for ObjC property name to be a single 
acronym. (authored by Wizard, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D46374

Files:
  clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
  clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration-custom.m
  clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration.m


Index: clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration.m
===
--- clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration.m
+++ clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration.m
@@ -21,6 +21,7 @@
 @property(assign, nonatomic) int enable2GBackgroundFetch;
 @property(assign, nonatomic) int shouldUseCFPreferences;
 @property(assign, nonatomic) int enableGLAcceleration;
+@property(assign, nonatomic) int ID;
 @end
 
 @interface Foo (Bar)
Index: 
clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration-custom.m
===
--- clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration-custom.m
+++ clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration-custom.m
@@ -14,4 +14,5 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'ABC_custom_prefix' 
not using lowerCamelCase style or not prefixed in a category, according to the 
Apple Coding Guidelines [objc-property-declaration]
 @property(assign, nonatomic) int GIFIgnoreStandardAcronym;
 // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 
'GIFIgnoreStandardAcronym' not using lowerCamelCase style or not prefixed in a 
category, according to the Apple Coding Guidelines [objc-property-declaration]
+@property(strong, nonatomic) NSString *TGIF;
 @end
Index: clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -217,6 +217,12 @@
   assert(MatchedDecl->getName().size() > 0);
   auto *DeclContext = MatchedDecl->getDeclContext();
   auto *CategoryDecl = llvm::dyn_cast(DeclContext);
+
+  auto AcronymsRegex =
+  llvm::Regex("^" + AcronymsGroupRegex(EscapedAcronyms) + "$");
+  if (AcronymsRegex.match(MatchedDecl->getName())) {
+return;
+  }
   if (CategoryDecl != nullptr &&
   hasCategoryPropertyPrefix(MatchedDecl->getName())) {
 if (!prefixedPropertyNameValid(MatchedDecl->getName(), EscapedAcronyms) ||


Index: clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration.m
===
--- clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration.m
+++ clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration.m
@@ -21,6 +21,7 @@
 @property(assign, nonatomic) int enable2GBackgroundFetch;
 @property(assign, nonatomic) int shouldUseCFPreferences;
 @property(assign, nonatomic) int enableGLAcceleration;
+@property(assign, nonatomic) int ID;
 @end
 
 @interface Foo (Bar)
Index: clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration-custom.m
===
--- clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration-custom.m
+++ clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration-custom.m
@@ -14,4 +14,5 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'ABC_custom_prefix' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
 @property(assign, nonatomic) int GIFIgnoreStandardAcronym;
 // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'GIFIgnoreStandardAcronym' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
+@property(strong, nonatomic) NSString *TGIF;
 @end
Index: clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -217,6 +217,12 @@
   assert(MatchedDecl->getName().size() > 0);
   auto *DeclContext = MatchedDecl->getDeclContext();
   auto *CategoryDecl = llvm::dyn_cast(DeclContext);
+
+  auto AcronymsRegex =
+  llvm::Regex("^" + AcronymsGroupRegex(EscapedAcronyms) + "$");
+  if (AcronymsRegex.match(MatchedDecl->getName())) {
+return;
+  }
   if (CategoryDecl != nullptr &&
   hasCategoryPropertyPrefix(MatchedDecl->getName())) {
 if (!prefixedPropertyNameValid(MatchedDecl->getName(), EscapedAcron

[clang-tools-extra] r331545 - Add support for ObjC property name to be a single acronym.

[Yan Zhang via cfe-commits]

Author: wizard
Date: Fri May  4 11:14:08 2018
New Revision: 331545

URL: http://llvm.org/viewvc/llvm-project?rev=331545&view=rev
Log:
Add support for ObjC property name to be a single acronym.

Summary:
This change will support cases like:

```
@property(assign, nonatomic) int ID;
```

Reviewers: benhamilton, hokein

Reviewed By: benhamilton

Subscribers: klimek, cfe-commits

Tags: #clang-tools-extra

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

Modified:
clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration-custom.m
clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration.m

Modified: clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp?rev=331545&r1=331544&r2=331545&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp Fri 
May  4 11:14:08 2018
@@ -217,6 +217,12 @@ void PropertyDeclarationCheck::check(con
   assert(MatchedDecl->getName().size() > 0);
   auto *DeclContext = MatchedDecl->getDeclContext();
   auto *CategoryDecl = llvm::dyn_cast(DeclContext);
+
+  auto AcronymsRegex =
+  llvm::Regex("^" + AcronymsGroupRegex(EscapedAcronyms) + "$");
+  if (AcronymsRegex.match(MatchedDecl->getName())) {
+return;
+  }
   if (CategoryDecl != nullptr &&
   hasCategoryPropertyPrefix(MatchedDecl->getName())) {
 if (!prefixedPropertyNameValid(MatchedDecl->getName(), EscapedAcronyms) ||

Modified: 
clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration-custom.m
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration-custom.m?rev=331545&r1=331544&r2=331545&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration-custom.m 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration-custom.m 
Fri May  4 11:14:08 2018
@@ -14,4 +14,5 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'ABC_custom_prefix' 
not using lowerCamelCase style or not prefixed in a category, according to the 
Apple Coding Guidelines [objc-property-declaration]
 @property(assign, nonatomic) int GIFIgnoreStandardAcronym;
 // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 
'GIFIgnoreStandardAcronym' not using lowerCamelCase style or not prefixed in a 
category, according to the Apple Coding Guidelines [objc-property-declaration]
+@property(strong, nonatomic) NSString *TGIF;
 @end

Modified: clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration.m
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration.m?rev=331545&r1=331544&r2=331545&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration.m 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration.m Fri May 
 4 11:14:08 2018
@@ -21,6 +21,7 @@
 @property(assign, nonatomic) int enable2GBackgroundFetch;
 @property(assign, nonatomic) int shouldUseCFPreferences;
 @property(assign, nonatomic) int enableGLAcceleration;
+@property(assign, nonatomic) int ID;
 @end
 
 @interface Foo (Bar)


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

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

[Paul Fultz II via Phabricator via cfe-commits]

pfultz2 updated this revision to Diff 145229.
pfultz2 added a comment.

Moved flag for alpha checks to the ClangTidyContext


https://reviews.llvm.org/D46159

Files:
  clang-tidy/ClangTidy.cpp
  clang-tidy/ClangTidy.h
  clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tidy/tool/ClangTidyMain.cpp
  test/clang-tidy/enable-alpha-checks.cpp

Index: test/clang-tidy/enable-alpha-checks.cpp
===
--- /dev/null
+++ test/clang-tidy/enable-alpha-checks.cpp
@@ -0,0 +1,6 @@
+// Check if '-allow-enabling-analyzer-alpha-checkers' is visible for users
+// RUN: clang-tidy -help | not grep 'allow-enabling-analyzer-alpha-checkers'
+
+// Check if '-allow-enabling-analyzer-alpha-checkers' enables alpha checks.
+// RUN: clang-tidy -checks=* -list-checks | not grep 'clang-analyzer-alpha'
+// RUN: clang-tidy -checks=* -list-checks -allow-enabling-analyzer-alpha-checkers | grep 'clang-analyzer-alpha'
Index: clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tidy/tool/ClangTidyMain.cpp
@@ -192,6 +192,14 @@
cl::init(false),
cl::cat(ClangTidyCategory));
 
+/// This option allows enabling alpha checkers from the static analyzer, that
+/// are experimental. This option is set to false and not visible in help,
+/// because it is highly not recommended for users.
+static cl::opt
+AllowEnablingAnalyzerAlphaCheckers("allow-enabling-analyzer-alpha-checkers",
+   cl::init(false), cl::Hidden,
+   cl::cat(ClangTidyCategory));
+
 static cl::opt ExportFixes("export-fixes", cl::desc(R"(
 YAML file to store suggested fixes in. The
 stored fixes can be applied to the input source
@@ -388,7 +396,8 @@
  << EC.message() << "\n";
   }
   ClangTidyOptions EffectiveOptions = OptionsProvider->getOptions(FilePath);
-  std::vector EnabledChecks = getCheckNames(EffectiveOptions);
+  std::vector EnabledChecks =
+  getCheckNames(EffectiveOptions, AllowEnablingAnalyzerAlphaCheckers);
 
   if (ExplainConfig) {
 // FIXME: Show other ClangTidyOptions' fields, like ExtraArg.
@@ -419,7 +428,8 @@
   }
 
   if (DumpConfig) {
-EffectiveOptions.CheckOptions = getCheckOptions(EffectiveOptions);
+EffectiveOptions.CheckOptions =
+getCheckOptions(EffectiveOptions, AllowEnablingAnalyzerAlphaCheckers);
 llvm::outs() << configurationAsText(
 ClangTidyOptions::getDefaults().mergeWith(
 EffectiveOptions))
@@ -444,7 +454,8 @@
   llvm::InitializeAllTargetMCs();
   llvm::InitializeAllAsmParsers();
 
-  ClangTidyContext Context(std::move(OwningOptionsProvider));
+  ClangTidyContext Context(std::move(OwningOptionsProvider),
+   AllowEnablingAnalyzerAlphaCheckers);
   runClangTidy(Context, OptionsParser.getCompilations(), PathList, BaseFS,
EnableCheckProfile ? &Profile : nullptr);
   ArrayRef Errors = Context.getErrors();
Index: clang-tidy/ClangTidyDiagnosticConsumer.h
===
--- clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -104,7 +104,8 @@
 class ClangTidyContext {
 public:
   /// \brief Initializes \c ClangTidyContext instance.
-  ClangTidyContext(std::unique_ptr OptionsProvider);
+  ClangTidyContext(std::unique_ptr OptionsProvider,
+   bool AllowEnablingAnalyzerAlphaCheckers = false);
 
   ~ClangTidyContext();
 
@@ -186,6 +187,11 @@
 return CurrentBuildDirectory;
   }
 
+  /// \brief Turns on experimental alpha checkers from the static analyzer.
+  bool isAlphaChecksAllowed() const {
+return AllowEnablingAnalyzerAlphaCheckers;
+  }
+
 private:
   // Calls setDiagnosticsEngine() and storeError().
   friend class ClangTidyDiagnosticConsumer;
@@ -217,6 +223,8 @@
   llvm::DenseMap CheckNamesByDiagnosticID;
 
   ProfileData *Profile;
+
+  bool AllowEnablingAnalyzerAlphaCheckers;
 };
 
 /// \brief A diagnostic consumer that turns each \c Diagnostic into a
Index: clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -177,9 +177,11 @@
 };
 
 ClangTidyContext::ClangTidyContext(
-std::unique_ptr OptionsProvider)
+std::unique_ptr OptionsProvider,
+bool AllowEnablingAnalyzerAlphaCheckers)
 : DiagEngine(nullptr), OptionsProvider(std::move(OptionsProvider)),
-  Profile(nullptr) {
+  Profile(nullptr),
+  AllowEnablingAnalyzerAlphaCheckers(AllowEnablingAnalyzerAlphaCheckers) {
   // Before the first translation unit we can get errors related to command-line
   // parsing, use empty string for th

[PATCH] D46435: [x86] Introduce the encl[u|s|v] intrinsics

[Craig Topper via Phabricator via cfe-commits]

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

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D46435



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

[PATCH] D46431: [x86] Introduce the pconfig intrinsic

[Craig Topper via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D46431



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

[PATCH] D46443: Add missing cstdalign header

[Sam Clegg via Phabricator via cfe-commits]

sbc100 created this revision.
Herald added subscribers: cfe-commits, christof, aheejin.

Repository:
  rCXX libc++

https://reviews.llvm.org/D46443

Files:
  include/cassert
  include/cstdalign
  test/libcxx/min_max_macros.sh.cpp
  test/libcxx/utilities/any/size_and_alignment.pass.cpp


Index: test/libcxx/utilities/any/size_and_alignment.pass.cpp
===
--- test/libcxx/utilities/any/size_and_alignment.pass.cpp
+++ test/libcxx/utilities/any/size_and_alignment.pass.cpp
@@ -14,6 +14,7 @@
 // Check that the size and alignment of any are what we expect.
 
 #include 
+#include 
 
 int main()
 {
Index: test/libcxx/min_max_macros.sh.cpp
===
--- test/libcxx/min_max_macros.sh.cpp
+++ test/libcxx/min_max_macros.sh.cpp
@@ -70,6 +70,8 @@
 TEST_MACROS();
 #include 
 TEST_MACROS();
+#include 
+TEST_MACROS();
 #include 
 TEST_MACROS();
 #include 
Index: include/cstdalign
===
--- include/cstdalign
+++ include/cstdalign
@@ -1,25 +1,33 @@
 // -*- C++ -*-
-//===-- cassert 
---===//
+//===--- cstdalign 
===//
 //
 // The LLVM Compiler Infrastructure
 //
 // This file is dual licensed under the MIT and the University of Illinois Open
 // Source Licenses. See LICENSE.TXT for details.
 //
 
//===--===//
 
+#ifndef _LIBCPP_CSTDALIGN
+#define _LIBCPP_CSTDALIGN
+
 /*
-cassert synopsis
+cstdalign synopsis
 
 Macros:
 
-assert
+   alignof
+   alignas
+__alignof_is_defined
+__alignas_is_defined
 
 */
 
 #include <__config>
-#include 
+#include 
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
 #endif
+
+#endif  // _LIBCPP_CSTDALIGN
Index: include/cassert
===
--- include/cassert
+++ include/cassert
@@ -8,6 +8,9 @@
 //
 
//===--===//
 
+#ifndef _LIBCPP_CASSERT
+#define _LIBCPP_CASSERT
+
 /*
 cassert synopsis
 
@@ -23,3 +26,5 @@
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
 #endif
+
+#endif  // _LIBCPP_CASSERT


Index: test/libcxx/utilities/any/size_and_alignment.pass.cpp
===
--- test/libcxx/utilities/any/size_and_alignment.pass.cpp
+++ test/libcxx/utilities/any/size_and_alignment.pass.cpp
@@ -14,6 +14,7 @@
 // Check that the size and alignment of any are what we expect.
 
 #include 
+#include 
 
 int main()
 {
Index: test/libcxx/min_max_macros.sh.cpp
===
--- test/libcxx/min_max_macros.sh.cpp
+++ test/libcxx/min_max_macros.sh.cpp
@@ -70,6 +70,8 @@
 TEST_MACROS();
 #include 
 TEST_MACROS();
+#include 
+TEST_MACROS();
 #include 
 TEST_MACROS();
 #include 
Index: include/cstdalign
===
--- include/cstdalign
+++ include/cstdalign
@@ -1,25 +1,33 @@
 // -*- C++ -*-
-//===-- cassert ---===//
+//===--- cstdalign ===//
 //
 // The LLVM Compiler Infrastructure
 //
 // This file is dual licensed under the MIT and the University of Illinois Open
 // Source Licenses. See LICENSE.TXT for details.
 //
 //===--===//
 
+#ifndef _LIBCPP_CSTDALIGN
+#define _LIBCPP_CSTDALIGN
+
 /*
-cassert synopsis
+cstdalign synopsis
 
 Macros:
 
-assert
+   alignof
+   alignas
+__alignof_is_defined
+__alignas_is_defined
 
 */
 
 #include <__config>
-#include 
+#include 
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
 #endif
+
+#endif  // _LIBCPP_CSTDALIGN
Index: include/cassert
===
--- include/cassert
+++ include/cassert
@@ -8,6 +8,9 @@
 //
 //===--===//
 
+#ifndef _LIBCPP_CASSERT
+#define _LIBCPP_CASSERT
+
 /*
 cassert synopsis
 
@@ -23,3 +26,5 @@
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
 #endif
+
+#endif  // _LIBCPP_CASSERT
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46403: [CFI] Force LLVM to die if the implicit blacklist files cannot be found.

[Caroline Tice via Phabricator via cfe-commits]

cmtice updated this revision to Diff 145243.
cmtice added a comment.

Updated the error to only occur for CFI blacklist, and added test case.


https://reviews.llvm.org/D46403

Files:
  lib/Driver/SanitizerArgs.cpp
  test/Driver/fsanitize-blacklist.c


Index: test/Driver/fsanitize-blacklist.c
===
--- test/Driver/fsanitize-blacklist.c
+++ test/Driver/fsanitize-blacklist.c
@@ -62,4 +62,8 @@
 // CHECK-ONLY-FIRST-DISABLED: -fsanitize-blacklist={{.*}}.second
 // CHECK-ONLY_FIRST-DISABLED-NOT: good
 
+// If cfi_blacklist.txt cannot be found in the resource dir, driver should 
fail.
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi -resource-dir=/dev/null 
%s -### 2>&1 | FileCheck %s --check-prefix=CHECK-MISSING-CFI-BLACKLIST
+// CHECK-MISSING-CFI-BLACKLIST: error: no such file or directory: 
'{{.*}}/share/cfi_blacklist.txt'
+
 // DELIMITERS: {{^ *"}}
Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -115,6 +115,8 @@
 llvm::sys::path::append(Path, "share", BL.File);
 if (llvm::sys::fs::exists(Path))
   BlacklistFiles.push_back(Path.str());
+else if (BL.Mask == CFI)
+  D.Diag(clang::diag::err_drv_no_such_file) << Path;
   }
 }
 


Index: test/Driver/fsanitize-blacklist.c
===
--- test/Driver/fsanitize-blacklist.c
+++ test/Driver/fsanitize-blacklist.c
@@ -62,4 +62,8 @@
 // CHECK-ONLY-FIRST-DISABLED: -fsanitize-blacklist={{.*}}.second
 // CHECK-ONLY_FIRST-DISABLED-NOT: good
 
+// If cfi_blacklist.txt cannot be found in the resource dir, driver should fail.
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi -resource-dir=/dev/null %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-MISSING-CFI-BLACKLIST
+// CHECK-MISSING-CFI-BLACKLIST: error: no such file or directory: '{{.*}}/share/cfi_blacklist.txt'
+
 // DELIMITERS: {{^ *"}}
Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -115,6 +115,8 @@
 llvm::sys::path::append(Path, "share", BL.File);
 if (llvm::sys::fs::exists(Path))
   BlacklistFiles.push_back(Path.str());
+else if (BL.Mask == CFI)
+  D.Diag(clang::diag::err_drv_no_such_file) << Path;
   }
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46443: Add missing cstdalign header

[Sam Clegg via Phabricator via cfe-commits]

sbc100 updated this revision to Diff 145247.
sbc100 added a comment.

- revert


Repository:
  rCXX libc++

https://reviews.llvm.org/D46443

Files:
  include/cstdalign
  test/libcxx/min_max_macros.sh.cpp
  test/libcxx/utilities/any/size_and_alignment.pass.cpp


Index: test/libcxx/utilities/any/size_and_alignment.pass.cpp
===
--- test/libcxx/utilities/any/size_and_alignment.pass.cpp
+++ test/libcxx/utilities/any/size_and_alignment.pass.cpp
@@ -14,6 +14,7 @@
 // Check that the size and alignment of any are what we expect.
 
 #include 
+#include 
 
 int main()
 {
Index: test/libcxx/min_max_macros.sh.cpp
===
--- test/libcxx/min_max_macros.sh.cpp
+++ test/libcxx/min_max_macros.sh.cpp
@@ -70,6 +70,8 @@
 TEST_MACROS();
 #include 
 TEST_MACROS();
+#include 
+TEST_MACROS();
 #include 
 TEST_MACROS();
 #include 
Index: include/cstdalign
===
--- /dev/null
+++ include/cstdalign
@@ -0,0 +1,33 @@
+// -*- C++ -*-
+//===--- cstdalign 
===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef _LIBCPP_CSTDALIGN
+#define _LIBCPP_CSTDALIGN
+
+/*
+cstdalign synopsis
+
+Macros:
+
+   alignof
+   alignas
+__alignof_is_defined
+__alignas_is_defined
+
+*/
+
+#include <__config>
+#include 
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#pragma GCC system_header
+#endif
+
+#endif  // _LIBCPP_CSTDALIGN


Index: test/libcxx/utilities/any/size_and_alignment.pass.cpp
===
--- test/libcxx/utilities/any/size_and_alignment.pass.cpp
+++ test/libcxx/utilities/any/size_and_alignment.pass.cpp
@@ -14,6 +14,7 @@
 // Check that the size and alignment of any are what we expect.
 
 #include 
+#include 
 
 int main()
 {
Index: test/libcxx/min_max_macros.sh.cpp
===
--- test/libcxx/min_max_macros.sh.cpp
+++ test/libcxx/min_max_macros.sh.cpp
@@ -70,6 +70,8 @@
 TEST_MACROS();
 #include 
 TEST_MACROS();
+#include 
+TEST_MACROS();
 #include 
 TEST_MACROS();
 #include 
Index: include/cstdalign
===
--- /dev/null
+++ include/cstdalign
@@ -0,0 +1,33 @@
+// -*- C++ -*-
+//===--- cstdalign ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef _LIBCPP_CSTDALIGN
+#define _LIBCPP_CSTDALIGN
+
+/*
+cstdalign synopsis
+
+Macros:
+
+   alignof
+   alignas
+__alignof_is_defined
+__alignas_is_defined
+
+*/
+
+#include <__config>
+#include 
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#pragma GCC system_header
+#endif
+
+#endif  // _LIBCPP_CSTDALIGN
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

OK, As I see it, we have two choices:

(1) Stash the expressions in Sema and import them like

In https://reviews.llvm.org/D45476#1088047, @rjmccall wrote:

>




> Ah.  If you want to be able to find this thing without a Sema around in order 
> to
>  handle deserialized expressions by just caching things in the ASTContext, 
> yes,
>  I agree that it would be difficult to build a `CXXConstructExpr` correctly.  
> I don't
>  fully understand the goal of avoiding serialization here, though.

Perhaps I don't fully understand the goal of avoiding serialization either.

>>> STLs *frequently* make use of default arguments on copy constructors (for
>>>  allocators). I agree that it’s unlikely that that would happen here, but
>>>  that’s precisely because it’s unlikely that this type would ever be
>>>  non-trivial.
>> 
>> A couple of points. First, when I say copy constructor, I mean the special 
>> member, which
>>  cannot have default arguments,
> 
> I'm sorry, but this is just not true.  The formation rules for a copy 
> constructor are laid
>  out in [class.copy]p2, and they explicitly allow default arguments.

Don't apologize because I'm wrong :-P. Thanks for the correction.

>> Second, in the synopsis for the STL types, no constructors are declared. 
>> Although I don't
>>  think the standard spells it out anywhere, I believe this forbids the types 
>> from having user
>>  defined constructors (though perhaps not user-declared explicitly defaulted 
>> constructors.
>>  For example adding a user defined destructor would be non-conforming since 
>> it makes
>>  the types non-literal).
> 
> That would certainly be helpful, but I don't think it's true; in general the 
> standard describes
>  what things are guaranteed to work with library types, but it doesn't 
> generally constrain
>  the existence of other operations.

I think this is somewhat of a moot point, but I think the constraint is in 
`[member.functions]p2`:

> For a non-virtual member function described in the C++ standard library, an 
> implementation may declare
>  a different set of member function signatures, provided that any call to the 
> member function that would
>  select an overload from the set of declarations described in this document 
> behaves as if that overload were selected.

My argument is that because the class synopsis for the comparison category 
types doesn't describe or declare
the copy constructor, so the implementation *isn't*  free to declare it 
differently.


https://reviews.llvm.org/D45476



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

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

This is significantly more complexity than we need. We're talking about 
constexpr variables here, so we just need a `VarDecl* -- you can then ask that 
declaration for its evaluated value and emit that directly.


https://reviews.llvm.org/D45476



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

[PATCH] D46374: Add support for ObjC property name to be a single acronym.

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

Looks good to me.


Repository:
  rL LLVM

https://reviews.llvm.org/D46374



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

[PATCH] D46446: [c++17] Fix assertion on synthesizing deduction guides after a fatal error.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai created this revision.
vsapsai added a reviewer: rsmith.

After a fatal error Sema::InstantiatingTemplate doesn't allow further
instantiation and doesn't push a CodeSynthesisContext. When we tried to
synthesize implicit deduction guides from constructors we hit the
assertion

> Assertion failed: (!CodeSynthesisContexts.empty() && "Cannot perform an 
> instantiation without some context on the " "instantiation stack"), function 
> SubstType, file clang/lib/Sema/SemaTemplateInstantiate.cpp, line 1580.

Fix by avoiding deduction guide synthesis if InstantiatingTemplate is invalid.

rdar://problem/39051732


https://reviews.llvm.org/D46446

Files:
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/SemaTemplate/instantiate-after-fatal-cxx17.cpp


Index: clang/test/SemaTemplate/instantiate-after-fatal-cxx17.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/instantiate-after-fatal-cxx17.cpp
@@ -0,0 +1,16 @@
+// RUN: not %clang_cc1 -std=c++17 -fsyntax-only -ferror-limit 1 %s 2>&1 | 
FileCheck %s
+
+#error Error 1
+#error Error 2
+// CHECK: fatal error: too many errors emitted, stopping now
+
+namespace rdar39051732 {
+
+  template struct A {
+template  A(T&, ...);
+  };
+  // Deduction guide triggers constructor instantiation.
+  template A(const T&, const T&) -> A;
+
+}
+
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -1935,6 +1935,8 @@
   // FIXME: Add a kind for this to give more meaningful diagnostics. But can
   // this substitution process actually fail?
   InstantiatingTemplate BuildingDeductionGuides(*this, Loc, Template);
+  if (BuildingDeductionGuides.isInvalid())
+return;
 
   // Convert declared constructors into deduction guide templates.
   // FIXME: Skip constructors for which deduction must necessarily fail (those


Index: clang/test/SemaTemplate/instantiate-after-fatal-cxx17.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/instantiate-after-fatal-cxx17.cpp
@@ -0,0 +1,16 @@
+// RUN: not %clang_cc1 -std=c++17 -fsyntax-only -ferror-limit 1 %s 2>&1 | FileCheck %s
+
+#error Error 1
+#error Error 2
+// CHECK: fatal error: too many errors emitted, stopping now
+
+namespace rdar39051732 {
+
+  template struct A {
+template  A(T&, ...);
+  };
+  // Deduction guide triggers constructor instantiation.
+  template A(const T&, const T&) -> A;
+
+}
+
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -1935,6 +1935,8 @@
   // FIXME: Add a kind for this to give more meaningful diagnostics. But can
   // this substitution process actually fail?
   InstantiatingTemplate BuildingDeductionGuides(*this, Loc, Template);
+  if (BuildingDeductionGuides.isInvalid())
+return;
 
   // Convert declared constructors into deduction guide templates.
   // FIXME: Skip constructors for which deduction must necessarily fail (those
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46403: [CFI] Force LLVM to die if the implicit blacklist files cannot be found.

[Vedant Kumar via Phabricator via cfe-commits]

vsk added inline comments.



Comment at: lib/Driver/SanitizerArgs.cpp:118
   BlacklistFiles.push_back(Path.str());
+else if (BL.Mask == CFI)
+  D.Diag(clang::diag::err_drv_no_such_file) << Path;

CFI can be enabled with other sanitizers, such as ubsan. I think you'll need 
'Mask & CFI' here. It'd be great to have a test for this case, too (i.e 
-fsanitize=cfi,undefined -resource-dir=/dev/null should also give you this 
diagnostic).



Comment at: test/Driver/fsanitize-blacklist.c:65
 
+// If cfi_blacklist.txt cannot be found in the resource dir, driver should 
fail.
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi -resource-dir=/dev/null 
%s -### 2>&1 | FileCheck %s --check-prefix=CHECK-MISSING-CFI-BLACKLIST

It'd be more helpful for readers if this comment were in the source, at the 
point where the diagnostic is emitted.


https://reviews.llvm.org/D46403



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

[PATCH] D46403: [CFI] Force LLVM to die if the implicit blacklist files cannot be found.

[Vedant Kumar via Phabricator via cfe-commits]

vsk added inline comments.



Comment at: lib/Driver/SanitizerArgs.cpp:118
   BlacklistFiles.push_back(Path.str());
+else if (BL.Mask == CFI)
+  D.Diag(clang::diag::err_drv_no_such_file) << Path;

vsk wrote:
> CFI can be enabled with other sanitizers, such as ubsan. I think you'll need 
> 'Mask & CFI' here. It'd be great to have a test for this case, too (i.e 
> -fsanitize=cfi,undefined -resource-dir=/dev/null should also give you this 
> diagnostic).
Sorry, I misread the surrounding code. Your check is fine as-is.


https://reviews.llvm.org/D46403



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

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D45476#1088180, @EricWF wrote:

> OK, As I see it, we have two choices:
>
> (1) Stash the expressions in Sema and import them like
>
> In https://reviews.llvm.org/D45476#1088047, @rjmccall wrote:
>
> > I'm sorry, but this is just not true.  The formation rules for a copy 
> > constructor are laid
> >  out in [class.copy]p2, and they explicitly allow default arguments.
>
>
> Don't apologize because I'm wrong :-P. Thanks for the correction.


It's been a thorn in my side quite a bit, too. :)

>>> Second, in the synopsis for the STL types, no constructors are declared. 
>>> Although I don't
>>>  think the standard spells it out anywhere, I believe this forbids the 
>>> types from having user
>>>  defined constructors (though perhaps not user-declared explicitly 
>>> defaulted constructors.
>>>  For example adding a user defined destructor would be non-conforming since 
>>> it makes
>>>  the types non-literal).
>> 
>> That would certainly be helpful, but I don't think it's true; in general the 
>> standard describes
>>  what things are guaranteed to work with library types, but it doesn't 
>> generally constrain
>>  the existence of other operations.
> 
> I think this is somewhat of a moot point, but I think the constraint is in 
> `[member.functions]p2`:
> 
>> For a non-virtual member function described in the C++ standard library, an 
>> implementation may declare
>>  a different set of member function signatures, provided that any call to 
>> the member function that would
>>  select an overload from the set of declarations described in this document 
>> behaves as if that overload were selected.
> 
> My argument is that because the class synopsis for the comparison category 
> types doesn't describe or declare
>  the copy constructor, so the implementation *isn't*  free to declare it 
> differently.

The type has to allow itself to be constructed with an l-value (whether const 
or not)
of its own type in order to be CopyConstructible.  However, that's just a 
statement
about the *signatures* of the type's constructors; it doesn't say anything 
about whether
those constructors are user-defined, nor does it limit what other constructors 
might
be provided as long as they don't somehow prevent copy-construction from 
succeeding.
(And in somewhat obscure cases, constructing a T with an l-value of type T won't
even select a copy constructor, and that's legal under the standard, too.)  All 
that matters,
absent requirements about T being an aggregate or trivially-copyable type, is 
that the
construction is well-formed and that it has the effect of copying the value.

Anyway, I agree that this is moot.

John.


https://reviews.llvm.org/D45476



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

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D45476#1088189, @rsmith wrote:

> This is significantly more complexity than we need. We're talking about 
> constexpr variables here, so we just need a `VarDecl* -- you can then ask 
> that declaration for its evaluated value and emit that directly.


If these variables are required to satisfy the requirements for that, then I 
agree that that would be the simplest solution.


https://reviews.llvm.org/D45476



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

[PATCH] D46439: Fix incorrect packed aligned structure layout

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

This looks fine, but needs a test.




Comment at: lib/Sema/SemaDecl.cpp:15576
 
+// Handle attributes before checking for alignas underalignment.
+if (Attr)

More generally: "before checking the layout".



Comment at: lib/Sema/SemaDecl.cpp:15651
 }
   } else {
 ObjCIvarDecl **ClsFields =

Do we need to do any attribute processing in this Objective-C interface / 
implementation / category case?


Repository:
  rC Clang

https://reviews.llvm.org/D46439



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

[PATCH] D46403: [CFI] Force LLVM to die if the implicit blacklist files cannot be found.

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

One problem with this direction is that clang doesn't ship a cfi blacklist in 
its default install, so, this leaves cfi users with stock toolchains to fend 
for themselves. I think it'd be a good idea to ship a basic cfi blacklist in 
clang's resource dir to avoid inadvertent breakage.


https://reviews.llvm.org/D46403



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

[PATCH] D36610: [Tooling] Add option to getFullyQualifiedName using a custom PritingPolicy

[Mikhail Ramalho via Phabricator via cfe-commits]

mikhail.ramalho updated this revision to Diff 145255.
mikhail.ramalho added a comment.

Fixed the test case.


https://reviews.llvm.org/D36610

Files:
  include/clang/AST/QualTypeNames.h
  lib/AST/QualTypeNames.cpp
  unittests/Tooling/QualTypeNamesTest.cpp


Index: unittests/Tooling/QualTypeNamesTest.cpp
===
--- unittests/Tooling/QualTypeNamesTest.cpp
+++ unittests/Tooling/QualTypeNamesTest.cpp
@@ -26,9 +26,13 @@
 std::string ExpectedName =
 ExpectedQualTypeNames.lookup(VD->getNameAsString());
 if (ExpectedName != "") {
-  std::string ActualName =
-  TypeName::getFullyQualifiedName(VD->getType(), *Context,
-  WithGlobalNsPrefix);
+  PrintingPolicy Policy(Context->getPrintingPolicy());
+  Policy.SuppressScope = false;
+  Policy.AnonymousTagLocations = true;
+  Policy.PolishForDeclaration = true;
+  Policy.SuppressUnwrittenScope = true;
+  std::string ActualName = TypeName::getFullyQualifiedName(
+  VD->getType(), *Context, Policy, WithGlobalNsPrefix);
   if (ExpectedName != ActualName) {
 // A custom message makes it much easier to see what declaration
 // failed compared to EXPECT_EQ.
@@ -217,6 +221,26 @@
   "  }\n"
   "}\n"
   );
+
+  TypeNameVisitor AnonStrucs;
+  AnonStrucs.ExpectedQualTypeNames["a"] = "short";
+  AnonStrucs.ExpectedQualTypeNames["un_in_st_1"] =
+  "union (anonymous struct at input.cc:1:1)::(anonymous union at "
+  "input.cc:2:27)";
+  AnonStrucs.ExpectedQualTypeNames["b"] = "short";
+  AnonStrucs.ExpectedQualTypeNames["un_in_st_2"] =
+  "union (anonymous struct at input.cc:1:1)::(anonymous union at "
+  "input.cc:5:27)";
+  AnonStrucs.ExpectedQualTypeNames["anon_st"] =
+  "struct (anonymous struct at input.cc:1:1)";
+  AnonStrucs.runOver(R"(struct {
+  union {
+short a;
+  } un_in_st_1;
+  union {
+short b;
+  } un_in_st_2;
+} anon_st;)");
 }
 
 }  // end anonymous namespace
Index: lib/AST/QualTypeNames.cpp
===
--- lib/AST/QualTypeNames.cpp
+++ lib/AST/QualTypeNames.cpp
@@ -452,12 +452,8 @@
 
 std::string getFullyQualifiedName(QualType QT,
   const ASTContext &Ctx,
+  const PrintingPolicy &Policy,
   bool WithGlobalNsPrefix) {
-  PrintingPolicy Policy(Ctx.getPrintingPolicy());
-  Policy.SuppressScope = false;
-  Policy.AnonymousTagLocations = false;
-  Policy.PolishForDeclaration = true;
-  Policy.SuppressUnwrittenScope = true;
   QualType FQQT = getFullyQualifiedType(QT, Ctx, WithGlobalNsPrefix);
   return FQQT.getAsString(Policy);
 }
Index: include/clang/AST/QualTypeNames.h
===
--- include/clang/AST/QualTypeNames.h
+++ include/clang/AST/QualTypeNames.h
@@ -72,6 +72,7 @@
 /// \param[in] WithGlobalNsPrefix - If true, then the global namespace
 /// specifier "::" will be prepended to the fully qualified name.
 std::string getFullyQualifiedName(QualType QT, const ASTContext &Ctx,
+  const PrintingPolicy &Policy,
   bool WithGlobalNsPrefix = false);
 
 /// \brief Generates a QualType that can be used to name the same type


Index: unittests/Tooling/QualTypeNamesTest.cpp
===
--- unittests/Tooling/QualTypeNamesTest.cpp
+++ unittests/Tooling/QualTypeNamesTest.cpp
@@ -26,9 +26,13 @@
 std::string ExpectedName =
 ExpectedQualTypeNames.lookup(VD->getNameAsString());
 if (ExpectedName != "") {
-  std::string ActualName =
-  TypeName::getFullyQualifiedName(VD->getType(), *Context,
-  WithGlobalNsPrefix);
+  PrintingPolicy Policy(Context->getPrintingPolicy());
+  Policy.SuppressScope = false;
+  Policy.AnonymousTagLocations = true;
+  Policy.PolishForDeclaration = true;
+  Policy.SuppressUnwrittenScope = true;
+  std::string ActualName = TypeName::getFullyQualifiedName(
+  VD->getType(), *Context, Policy, WithGlobalNsPrefix);
   if (ExpectedName != ActualName) {
 // A custom message makes it much easier to see what declaration
 // failed compared to EXPECT_EQ.
@@ -217,6 +221,26 @@
   "  }\n"
   "}\n"
   );
+
+  TypeNameVisitor AnonStrucs;
+  AnonStrucs.ExpectedQualTypeNames["a"] = "short";
+  AnonStrucs.ExpectedQualTypeNames["un_in_st_1"] =
+  "union (anonymous struct at input.cc:1:1)::(anonymous union at "
+  "input.cc:2:27)";
+  AnonStrucs.ExpectedQualTypeNames["b"] = "short";
+  AnonStrucs.ExpectedQualTypeNames["un_in_st_2"] =
+  "u

[PATCH] D46415: [analyzer] pr36458: Fix retrieved value cast for symbolic void pointers.

[Artem Dergachev via Phabricator via cfe-commits]

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

Fix test names. Add one more test, just to make sure it works.


https://reviews.llvm.org/D46415

Files:
  lib/StaticAnalyzer/Core/Store.cpp
  test/Analysis/casts.c


Index: test/Analysis/casts.c
===
--- test/Analysis/casts.c
+++ test/Analysis/casts.c
@@ -149,3 +149,25 @@
 
   clang_analyzer_eval(*((char *)y1) == *((char *) y3)); // 
expected-warning{{TRUE}}
 }
+
+void *getVoidPtr();
+
+void testCastVoidPtrToIntPtrThroughIntTypedAssignment() {
+  int *x;
+  (*((int *)(&x))) = (int)getVoidPtr();
+  *x = 1; // no-crash
+}
+
+void testCastUIntPtrToIntPtrThroughIntTypedAssignment() {
+  unsigned u;
+  int *x;
+  (*((int *)(&x))) = (int)&u;
+  *x = 1;
+  clang_analyzer_eval(u == 1); // expected-warning{{TRUE}}
+}
+
+void testCastVoidPtrToIntPtrThroughUIntTypedAssignment() {
+  int *x;
+  (*((int *)(&x))) = (int)(unsigned *)getVoidPtr();
+  *x = 1; // no-crash
+}
Index: lib/StaticAnalyzer/Core/Store.cpp
===
--- lib/StaticAnalyzer/Core/Store.cpp
+++ lib/StaticAnalyzer/Core/Store.cpp
@@ -378,6 +378,20 @@
   if (castTy.isNull() || V.isUnknownOrUndef())
 return V;
 
+  // When retrieving symbolic pointer and expecting a non-void pointer,
+  // wrap them into element regions of the expected type if necessary.
+  // SValBuilder::dispatchCast() doesn't do that, but it is necessary to
+  // make sure that the retrieved value makes sense, because there's no other
+  // cast in the AST that would tell us to cast it to the correct pointer type.
+  // We might need to do that for non-void pointers as well.
+  // FIXME: We really need a single good function to perform casts for us
+  // correctly every time we need it.
+  if (castTy->isPointerType() && !castTy->isVoidPointerType())
+if (const auto *SR = dyn_cast_or_null(V.getAsRegion()))
+  if (SR->getSymbol()->getType().getCanonicalType() !=
+  castTy.getCanonicalType())
+return loc::MemRegionVal(castRegion(SR, castTy));
+
   return svalBuilder.dispatchCast(V, castTy);
 }
 


Index: test/Analysis/casts.c
===
--- test/Analysis/casts.c
+++ test/Analysis/casts.c
@@ -149,3 +149,25 @@
 
   clang_analyzer_eval(*((char *)y1) == *((char *) y3)); // expected-warning{{TRUE}}
 }
+
+void *getVoidPtr();
+
+void testCastVoidPtrToIntPtrThroughIntTypedAssignment() {
+  int *x;
+  (*((int *)(&x))) = (int)getVoidPtr();
+  *x = 1; // no-crash
+}
+
+void testCastUIntPtrToIntPtrThroughIntTypedAssignment() {
+  unsigned u;
+  int *x;
+  (*((int *)(&x))) = (int)&u;
+  *x = 1;
+  clang_analyzer_eval(u == 1); // expected-warning{{TRUE}}
+}
+
+void testCastVoidPtrToIntPtrThroughUIntTypedAssignment() {
+  int *x;
+  (*((int *)(&x))) = (int)(unsigned *)getVoidPtr();
+  *x = 1; // no-crash
+}
Index: lib/StaticAnalyzer/Core/Store.cpp
===
--- lib/StaticAnalyzer/Core/Store.cpp
+++ lib/StaticAnalyzer/Core/Store.cpp
@@ -378,6 +378,20 @@
   if (castTy.isNull() || V.isUnknownOrUndef())
 return V;
 
+  // When retrieving symbolic pointer and expecting a non-void pointer,
+  // wrap them into element regions of the expected type if necessary.
+  // SValBuilder::dispatchCast() doesn't do that, but it is necessary to
+  // make sure that the retrieved value makes sense, because there's no other
+  // cast in the AST that would tell us to cast it to the correct pointer type.
+  // We might need to do that for non-void pointers as well.
+  // FIXME: We really need a single good function to perform casts for us
+  // correctly every time we need it.
+  if (castTy->isPointerType() && !castTy->isVoidPointerType())
+if (const auto *SR = dyn_cast_or_null(V.getAsRegion()))
+  if (SR->getSymbol()->getType().getCanonicalType() !=
+  castTy.getCanonicalType())
+return loc::MemRegionVal(castRegion(SR, castTy));
+
   return svalBuilder.dispatchCast(V, castTy);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r331552 - Allow modifying the PrintingPolicy for fully qualified names.

[Sterling Augustine via cfe-commits]

Author: saugustine
Date: Fri May  4 13:12:39 2018
New Revision: 331552

URL: http://llvm.org/viewvc/llvm-project?rev=331552&view=rev
Log:
Allow modifying the PrintingPolicy for fully qualified names.

Author: mikhail.rama...@gmail.com


Modified:
cfe/trunk/include/clang/AST/QualTypeNames.h
cfe/trunk/lib/AST/QualTypeNames.cpp
cfe/trunk/unittests/Tooling/QualTypeNamesTest.cpp

Modified: cfe/trunk/include/clang/AST/QualTypeNames.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/QualTypeNames.h?rev=331552&r1=331551&r2=331552&view=diff
==
--- cfe/trunk/include/clang/AST/QualTypeNames.h (original)
+++ cfe/trunk/include/clang/AST/QualTypeNames.h Fri May  4 13:12:39 2018
@@ -72,6 +72,7 @@ namespace TypeName {
 /// \param[in] WithGlobalNsPrefix - If true, then the global namespace
 /// specifier "::" will be prepended to the fully qualified name.
 std::string getFullyQualifiedName(QualType QT, const ASTContext &Ctx,
+  const PrintingPolicy &Policy,
   bool WithGlobalNsPrefix = false);
 
 /// \brief Generates a QualType that can be used to name the same type

Modified: cfe/trunk/lib/AST/QualTypeNames.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/QualTypeNames.cpp?rev=331552&r1=331551&r2=331552&view=diff
==
--- cfe/trunk/lib/AST/QualTypeNames.cpp (original)
+++ cfe/trunk/lib/AST/QualTypeNames.cpp Fri May  4 13:12:39 2018
@@ -452,12 +452,8 @@ QualType getFullyQualifiedType(QualType
 
 std::string getFullyQualifiedName(QualType QT,
   const ASTContext &Ctx,
+  const PrintingPolicy &Policy,
   bool WithGlobalNsPrefix) {
-  PrintingPolicy Policy(Ctx.getPrintingPolicy());
-  Policy.SuppressScope = false;
-  Policy.AnonymousTagLocations = false;
-  Policy.PolishForDeclaration = true;
-  Policy.SuppressUnwrittenScope = true;
   QualType FQQT = getFullyQualifiedType(QT, Ctx, WithGlobalNsPrefix);
   return FQQT.getAsString(Policy);
 }

Modified: cfe/trunk/unittests/Tooling/QualTypeNamesTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/QualTypeNamesTest.cpp?rev=331552&r1=331551&r2=331552&view=diff
==
--- cfe/trunk/unittests/Tooling/QualTypeNamesTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/QualTypeNamesTest.cpp Fri May  4 13:12:39 2018
@@ -26,9 +26,13 @@ struct TypeNameVisitor : TestVisitorgetNameAsString());
 if (ExpectedName != "") {
-  std::string ActualName =
-  TypeName::getFullyQualifiedName(VD->getType(), *Context,
-  WithGlobalNsPrefix);
+  PrintingPolicy Policy(Context->getPrintingPolicy());
+  Policy.SuppressScope = false;
+  Policy.AnonymousTagLocations = true;
+  Policy.PolishForDeclaration = true;
+  Policy.SuppressUnwrittenScope = true;
+  std::string ActualName = TypeName::getFullyQualifiedName(
+  VD->getType(), *Context, Policy, WithGlobalNsPrefix);
   if (ExpectedName != ActualName) {
 // A custom message makes it much easier to see what declaration
 // failed compared to EXPECT_EQ.
@@ -217,6 +221,26 @@ TEST(QualTypeNameTest, getFullyQualified
   "  }\n"
   "}\n"
   );
+
+  TypeNameVisitor AnonStrucs;
+  AnonStrucs.ExpectedQualTypeNames["a"] = "short";
+  AnonStrucs.ExpectedQualTypeNames["un_in_st_1"] =
+  "union (anonymous struct at input.cc:1:1)::(anonymous union at "
+  "input.cc:2:27)";
+  AnonStrucs.ExpectedQualTypeNames["b"] = "short";
+  AnonStrucs.ExpectedQualTypeNames["un_in_st_2"] =
+  "union (anonymous struct at input.cc:1:1)::(anonymous union at "
+  "input.cc:5:27)";
+  AnonStrucs.ExpectedQualTypeNames["anon_st"] =
+  "struct (anonymous struct at input.cc:1:1)";
+  AnonStrucs.runOver(R"(struct {
+  union {
+short a;
+  } un_in_st_1;
+  union {
+short b;
+  } un_in_st_2;
+} anon_st;)");
 }
 
 }  // end anonymous namespace


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

Re: [PATCH] D36610: [Tooling] Add option to getFullyQualifiedName using a custom PritingPolicy

[Sterling Augustine via cfe-commits]

Committed as r331552.

On Fri, May 4, 2018 at 12:43 PM, Mikhail Ramalho via Phabricator <
revi...@reviews.llvm.org> wrote:

> mikhail.ramalho updated this revision to Diff 145255.
> mikhail.ramalho added a comment.
>
> Fixed the test case.
>
>
> https://reviews.llvm.org/D36610
>
> Files:
>   include/clang/AST/QualTypeNames.h
>   lib/AST/QualTypeNames.cpp
>   unittests/Tooling/QualTypeNamesTest.cpp
>
>
> Index: unittests/Tooling/QualTypeNamesTest.cpp
> ===
> --- unittests/Tooling/QualTypeNamesTest.cpp
> +++ unittests/Tooling/QualTypeNamesTest.cpp
> @@ -26,9 +26,13 @@
>  std::string ExpectedName =
>  ExpectedQualTypeNames.lookup(VD->getNameAsString());
>  if (ExpectedName != "") {
> -  std::string ActualName =
> -  TypeName::getFullyQualifiedName(VD->getType(), *Context,
> -  WithGlobalNsPrefix);
> +  PrintingPolicy Policy(Context->getPrintingPolicy());
> +  Policy.SuppressScope = false;
> +  Policy.AnonymousTagLocations = true;
> +  Policy.PolishForDeclaration = true;
> +  Policy.SuppressUnwrittenScope = true;
> +  std::string ActualName = TypeName::getFullyQualifiedName(
> +  VD->getType(), *Context, Policy, WithGlobalNsPrefix);
>if (ExpectedName != ActualName) {
>  // A custom message makes it much easier to see what declaration
>  // failed compared to EXPECT_EQ.
> @@ -217,6 +221,26 @@
>"  }\n"
>"}\n"
>);
> +
> +  TypeNameVisitor AnonStrucs;
> +  AnonStrucs.ExpectedQualTypeNames["a"] = "short";
> +  AnonStrucs.ExpectedQualTypeNames["un_in_st_1"] =
> +  "union (anonymous struct at input.cc:1:1)::(anonymous union at "
> +  "input.cc:2:27)";
> +  AnonStrucs.ExpectedQualTypeNames["b"] = "short";
> +  AnonStrucs.ExpectedQualTypeNames["un_in_st_2"] =
> +  "union (anonymous struct at input.cc:1:1)::(anonymous union at "
> +  "input.cc:5:27)";
> +  AnonStrucs.ExpectedQualTypeNames["anon_st"] =
> +  "struct (anonymous struct at input.cc:1:1)";
> +  AnonStrucs.runOver(R"(struct {
> +  union {
> +short a;
> +  } un_in_st_1;
> +  union {
> +short b;
> +  } un_in_st_2;
> +} anon_st;)");
>  }
>
>  }  // end anonymous namespace
> Index: lib/AST/QualTypeNames.cpp
> ===
> --- lib/AST/QualTypeNames.cpp
> +++ lib/AST/QualTypeNames.cpp
> @@ -452,12 +452,8 @@
>
>  std::string getFullyQualifiedName(QualType QT,
>const ASTContext &Ctx,
> +  const PrintingPolicy &Policy,
>bool WithGlobalNsPrefix) {
> -  PrintingPolicy Policy(Ctx.getPrintingPolicy());
> -  Policy.SuppressScope = false;
> -  Policy.AnonymousTagLocations = false;
> -  Policy.PolishForDeclaration = true;
> -  Policy.SuppressUnwrittenScope = true;
>QualType FQQT = getFullyQualifiedType(QT, Ctx, WithGlobalNsPrefix);
>return FQQT.getAsString(Policy);
>  }
> Index: include/clang/AST/QualTypeNames.h
> ===
> --- include/clang/AST/QualTypeNames.h
> +++ include/clang/AST/QualTypeNames.h
> @@ -72,6 +72,7 @@
>  /// \param[in] WithGlobalNsPrefix - If true, then the global namespace
>  /// specifier "::" will be prepended to the fully qualified name.
>  std::string getFullyQualifiedName(QualType QT, const ASTContext &Ctx,
> +  const PrintingPolicy &Policy,
>bool WithGlobalNsPrefix = false);
>
>  /// \brief Generates a QualType that can be used to name the same type
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits