r366663 - [analyzer] Fix -Wunused-function in NDEBUG builds with #ifdef LLVM_DUMP_METHOD

[Fangrui Song via cfe-commits]

Author: maskray
Date: Sun Jul 21 21:14:09 2019
New Revision: 33

URL: http://llvm.org/viewvc/llvm-project?rev=33=rev
Log:
[analyzer] Fix -Wunused-function in NDEBUG builds with #ifdef LLVM_DUMP_METHOD

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=33=32=33=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Sun Jul 21 21:14:09 2019
@@ -108,7 +108,7 @@ public:
Data == X.Data;
   }
 
-  void dump() const;
+  LLVM_DUMP_METHOD void dump() const;
 };
 } // end anonymous namespace
 
@@ -135,7 +135,9 @@ static inline raw_ostream <<(ra
 
 } // namespace llvm
 
-LLVM_DUMP_METHOD void BindingKey::dump() const { llvm::errs() << *this; }
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+void BindingKey::dump() const { llvm::errs() << *this; }
+#endif
 
 
//===--===//
 // Actual Store type.


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

[PATCH] D64448: gsl::Owner/gsl::Pointer: Add implicit annotations for some std types

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D64448#1595022 , @mgehre wrote:

> In D64448#1595017 , @lebedev.ri 
> wrote:
>
> > Just passing-by thought:
> >  These attributes that are being added implicitly, it will be possible to 
> > differentiate
> >  whether an attribute was actually present in the code, or was added 
> > implicitly,
> >  say for clang-tidy check purposes?
> >  Is there some 'implicit' bit on these, or they have a location not in the 
> > source buffer?
>
>
> Thanks for the question! The implicitly added attributes have an invalid 
> source location.


Sounds good, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64448



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

[PATCH] D15032: [clang-tidy] new checker cppcoreguidelines-pro-lifetime

[Matthias Gehre via Phabricator via cfe-commits]

mgehre abandoned this revision.
mgehre added a comment.
Herald added subscribers: wuzish, dmgreen.

Upstreaming of this checker will happen with D63954 
 and following revisions.


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

https://reviews.llvm.org/D15032



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

[PATCH] D64448: gsl::Owner/gsl::Pointer: Add implicit annotations for some std types

[Matthias Gehre via Phabricator via cfe-commits]

mgehre added a comment.

In D64448#1595017 , @lebedev.ri wrote:

> Just passing-by thought:
>  These attributes that are being added implicitly, it will be possible to 
> differentiate
>  whether an attribute was actually present in the code, or was added 
> implicitly,
>  say for clang-tidy check purposes?
>  Is there some 'implicit' bit on these, or they have a location not in the 
> source buffer?


Thanks for the question! The implicitly added attributes have an invalid source 
location.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64448



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

[PATCH] D64448: gsl::Owner/gsl::Pointer: Add implicit annotations for some std types

[Matthias Gehre via Phabricator via cfe-commits]

mgehre updated this revision to Diff 211010.
mgehre added a comment.

- Remove type traits


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64448

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/SemaCXX/attr-gsl-owner-pointer-std.cpp

Index: clang/test/SemaCXX/attr-gsl-owner-pointer-std.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-gsl-owner-pointer-std.cpp
@@ -0,0 +1,126 @@
+// RUN: %clang_cc1 -ast-dump %s | \
+// RUN: FileCheck --implicit-check-not OwnerAttr --implicit-check-not PointerAttr %s
+
+// Test attribute inference for types in the standard library.
+namespace std {
+// Attributes are inferred for a (complete) class.
+class any {
+  // CHECK: CXXRecordDecl {{.*}} any
+  // CHECK: OwnerAttr {{.*}}
+};
+
+// Attributes are inferred for instantiations of a complete template.
+template 
+class vector {
+public:
+  class iterator {};
+  // CHECK: ClassTemplateDecl {{.*}} vector
+  // CHECK: OwnerAttr {{.*}}
+  // CHECK: CXXRecordDecl {{.*}} iterator
+  // CHECK: PointerAttr {{.*}}
+  // CHECK: ClassTemplateSpecializationDecl {{.*}} vector
+  // CHECK: TemplateArgument type 'int'
+  // CHECK: OwnerAttr
+  // CHECK: CXXRecordDecl {{.*}} iterator
+  // CHECK: PointerAttr {{.*}}
+};
+static_assert(sizeof(vector), "");   // Force instantiation.
+static_assert(sizeof(vector::iterator), ""); // Force instantiation.
+
+// If std::container::iterator is a using declaration, attributes are inferred
+// for the underlying class.
+template 
+class __set_iterator {};
+// CHECK: ClassTemplateDecl {{.*}} __set_iterator
+// CHECK: ClassTemplateSpecializationDecl {{.*}} __set_iterator
+// CHECK: TemplateArgument type 'int'
+// CHECK: PointerAttr
+
+template 
+class set {
+  // CHECK: ClassTemplateDecl {{.*}} set
+  // CHECK: OwnerAttr {{.*}}
+  // CHECK: ClassTemplateSpecializationDecl {{.*}} set
+  // CHECK: OwnerAttr {{.*}}
+public:
+  using iterator = __set_iterator;
+};
+static_assert(sizeof(set::iterator), ""); // Force instantiation.
+
+// If std::container::iterator is a typedef, attributes are inferred for the
+// underlying class.
+template 
+class __map_iterator {};
+// CHECK: ClassTemplateDecl {{.*}} __map_iterator
+// CHECK: ClassTemplateSpecializationDecl {{.*}} __map_iterator
+// CHECK: TemplateArgument type 'int'
+// CHECK: PointerAttr
+
+template 
+class map {
+  // CHECK: ClassTemplateDecl {{.*}} map
+  // CHECK: OwnerAttr {{.*}}
+  // CHECK: ClassTemplateSpecializationDecl {{.*}} map
+  // CHECK: OwnerAttr {{.*}}
+public:
+  typedef __map_iterator iterator;
+};
+static_assert(sizeof(map::iterator), ""); // Force instantiation.
+
+// Inline namespaces are ignored when checking if
+// the class lives in the std namespace.
+inline namespace inlinens {
+template 
+class __unordered_map_iterator {};
+// CHECK: ClassTemplateDecl {{.*}} __unordered_map_iterator
+// CHECK: ClassTemplateSpecializationDecl {{.*}} __unordered_map_iterator
+// CHECK: TemplateArgument type 'int'
+// CHECK: PointerAttr
+
+template 
+class unordered_map {
+  // CHECK: ClassTemplateDecl {{.*}} unordered_map
+  // CHECK: OwnerAttr {{.*}}
+  // CHECK: ClassTemplateSpecializationDecl {{.*}} unordered_map
+  // CHECK: OwnerAttr {{.*}}
+public:
+  typedef __unordered_map_iterator iterator;
+};
+static_assert(sizeof(unordered_map::iterator), ""); // Force instantiation.
+} // namespace inlinens
+
+// std::list has an implicit gsl::Owner attribute,
+// but explicit attributes take precedence.
+template 
+class [[gsl::Pointer]] list{};
+// CHECK: ClassTemplateDecl {{.*}} list
+// CHECK: PointerAttr {{.*}}
+// CHECK: ClassTemplateSpecializationDecl {{.*}} list
+// CHECK: PointerAttr {{.*}}
+
+static_assert(sizeof(list), ""); // Force instantiation.
+
+// Forward declared template (Owner).
+template <
+class CharT,
+class Traits>
+class basic_regex;
+// CHECK: ClassTemplateDecl {{.*}} basic_regex
+// CHECK: OwnerAttr {{.*}}
+
+// Forward declared template (Pointer).
+template 
+class reference_wrapper;
+// CHECK: ClassTemplateDecl {{.*}} reference_wrapper
+// CHECK: PointerAttr {{.*}}
+
+class some_unknown_type;
+// CHECK: CXXRecordDecl {{.*}} some_unknown_type
+
+} // namespace std
+
+namespace user {
+// If a class is not in the std namespace, we don't infer the attributes.
+class any {
+};
+} // namespace user
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -699,6 +699,7 @@
   }
 
   SemaRef.InstantiateAttrs(TemplateArgs, D, Typedef);
+  SemaRef.addDefaultGslPointerAttribute(Typedef);
 
   Typedef->setAccess(D->getAccess());
 
Index: 

[PATCH] D64448: gsl::Owner/gsl::Pointer: Add implicit annotations for some std types

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Just passing-by thought:
These attributes that are being added implicitly, it will be possible to 
differentiate
whether an attribute was actually present in the code, or was added implicitly,
say for clang-tidy check purposes?
Is there some 'implicit' bit on these, or they have a location not in the 
source buffer?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64448



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

[PATCH] D64448: gsl::Owner/gsl::Pointer: Add implicit annotations for some std types

[Matthias Gehre via Phabricator via cfe-commits]

mgehre added a comment.

This should fix all open comments.




Comment at: clang/include/clang/Basic/AttrDocs.td:4167-4168
 pointer/reference to the data owned by ``O``. The owned data is assumed to end
-its lifetime once the owning object's lifetime ends.
+its lifetime once the owning object's lifetime ends. The argument ``T`` is
+optional.
 

rsmith wrote:
> ... and what does the attribute mean when the argument is omitted?
I added text that it is currently ignored. We will use the argument in future 
analysis, but already add parsing of it for forward compatibility.
Per Aaron's request, this part moved to the parent PR, D63954.



Comment at: clang/test/SemaCXX/attr-gsl-owner-pointer.cpp:86
+
+// Test builtin annotation for std types.
+namespace std {

mgehre wrote:
> gribozavr wrote:
> > mgehre wrote:
> > > gribozavr wrote:
> > > > I feel like these tests would be better off in a separate file.
> > > > 
> > > > It would be also good to explain which exact library we are trying to 
> > > > imitate in this test. Different libraries use different coding patterns.
> > > This is imitating techniques from different libraries - all techniques 
> > > that this implementation supports.
> > > 
> > > To check if all techniques that this implementation supports are enough 
> > > for real standard library implementations,
> > > I use 
> > > https://github.com/mgehre/llvm-project/blob/lifetime-ci/lifetime-attr-test.cpp
> > >  against them. Various versions of libstdc++
> > > and libc++ passed. I will test MSVC standard library next. If they would 
> > > use a technique that this implementation does not support yet,
> > > I will add support for that and the corresponding test here.
> > > I might fix MSVC support (if needed) only in the following PR:
> > > This is imitating techniques from different libraries - all techniques 
> > > that this implementation supports.
> > 
> > Okay -- please add comments about each technique then (and ideally, which 
> > libraries use them). Right now (for me, who didn't write the patch), the 
> > test looks like it is testing inference for a bunch of types, not for a 
> > bunch of techniques -- the differences are subtle and non-obvious.
> Sure, I will add comments.
I moved the `std` tests to a different file.



Comment at: clang/test/SemaCXX/attr-gsl-owner-pointer.cpp:93
 
-// Complete template
+// Attributes are added to a instantiatons of a complete template.
 template 

gribozavr wrote:
> Sorry, but what do you mean by a "concrete template"?
> 
> "Attributes are inferred on class template instantiations."
It says "complete", and I mean "not forward declared".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64448



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

[PATCH] D64448: gsl::Owner/gsl::Pointer: Add implicit annotations for some std types

[Matthias Gehre via Phabricator via cfe-commits]

mgehre updated this revision to Diff 211009.
mgehre marked 12 inline comments as done.
mgehre added a comment.

- Merge branch 'lifetime-categories' into hardcode-lifetime-categories; Split 
test; Move test to AST dump; fix for comments;


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64448

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Basic/TypeTraits.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/SemaCXX/attr-gsl-owner-pointer-std.cpp

Index: clang/test/SemaCXX/attr-gsl-owner-pointer-std.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-gsl-owner-pointer-std.cpp
@@ -0,0 +1,126 @@
+// RUN: %clang_cc1 -ast-dump %s | \
+// RUN: FileCheck --implicit-check-not OwnerAttr --implicit-check-not PointerAttr %s
+
+// Test attribute inference for types in the standard library.
+namespace std {
+// Attributes are inferred for a (complete) class.
+class any {
+  // CHECK: CXXRecordDecl {{.*}} any
+  // CHECK: OwnerAttr {{.*}}
+};
+
+// Attributes are inferred for instantiations of a complete template.
+template 
+class vector {
+public:
+  class iterator {};
+  // CHECK: ClassTemplateDecl {{.*}} vector
+  // CHECK: OwnerAttr {{.*}}
+  // CHECK: CXXRecordDecl {{.*}} iterator
+  // CHECK: PointerAttr {{.*}}
+  // CHECK: ClassTemplateSpecializationDecl {{.*}} vector
+  // CHECK: TemplateArgument type 'int'
+  // CHECK: OwnerAttr
+  // CHECK: CXXRecordDecl {{.*}} iterator
+  // CHECK: PointerAttr {{.*}}
+};
+static_assert(sizeof(vector), "");   // Force instantiation.
+static_assert(sizeof(vector::iterator), ""); // Force instantiation.
+
+// If std::container::iterator is a using declaration, attributes are inferred
+// for the underlying class.
+template 
+class __set_iterator {};
+// CHECK: ClassTemplateDecl {{.*}} __set_iterator
+// CHECK: ClassTemplateSpecializationDecl {{.*}} __set_iterator
+// CHECK: TemplateArgument type 'int'
+// CHECK: PointerAttr
+
+template 
+class set {
+  // CHECK: ClassTemplateDecl {{.*}} set
+  // CHECK: OwnerAttr {{.*}}
+  // CHECK: ClassTemplateSpecializationDecl {{.*}} set
+  // CHECK: OwnerAttr {{.*}}
+public:
+  using iterator = __set_iterator;
+};
+static_assert(sizeof(set::iterator), ""); // Force instantiation.
+
+// If std::container::iterator is a typedef, attributes are inferred for the
+// underlying class.
+template 
+class __map_iterator {};
+// CHECK: ClassTemplateDecl {{.*}} __map_iterator
+// CHECK: ClassTemplateSpecializationDecl {{.*}} __map_iterator
+// CHECK: TemplateArgument type 'int'
+// CHECK: PointerAttr
+
+template 
+class map {
+  // CHECK: ClassTemplateDecl {{.*}} map
+  // CHECK: OwnerAttr {{.*}}
+  // CHECK: ClassTemplateSpecializationDecl {{.*}} map
+  // CHECK: OwnerAttr {{.*}}
+public:
+  typedef __map_iterator iterator;
+};
+static_assert(sizeof(map::iterator), ""); // Force instantiation.
+
+// Inline namespaces are ignored when checking if
+// the class lives in the std namespace.
+inline namespace inlinens {
+template 
+class __unordered_map_iterator {};
+// CHECK: ClassTemplateDecl {{.*}} __unordered_map_iterator
+// CHECK: ClassTemplateSpecializationDecl {{.*}} __unordered_map_iterator
+// CHECK: TemplateArgument type 'int'
+// CHECK: PointerAttr
+
+template 
+class unordered_map {
+  // CHECK: ClassTemplateDecl {{.*}} unordered_map
+  // CHECK: OwnerAttr {{.*}}
+  // CHECK: ClassTemplateSpecializationDecl {{.*}} unordered_map
+  // CHECK: OwnerAttr {{.*}}
+public:
+  typedef __unordered_map_iterator iterator;
+};
+static_assert(sizeof(unordered_map::iterator), ""); // Force instantiation.
+} // namespace inlinens
+
+// std::list has an implicit gsl::Owner attribute,
+// but explicit attributes take precedence.
+template 
+class [[gsl::Pointer]] list{};
+// CHECK: ClassTemplateDecl {{.*}} list
+// CHECK: PointerAttr {{.*}}
+// CHECK: ClassTemplateSpecializationDecl {{.*}} list
+// CHECK: PointerAttr {{.*}}
+
+static_assert(sizeof(list), ""); // Force instantiation.
+
+// Forward declared template (Owner).
+template <
+class CharT,
+class Traits>
+class basic_regex;
+// CHECK: ClassTemplateDecl {{.*}} basic_regex
+// CHECK: OwnerAttr {{.*}}
+
+// Forward declared template (Pointer).
+template 
+class reference_wrapper;
+// CHECK: ClassTemplateDecl {{.*}} reference_wrapper
+// CHECK: PointerAttr {{.*}}
+
+class some_unknown_type;
+// CHECK: CXXRecordDecl {{.*}} some_unknown_type
+
+} // namespace std
+
+namespace user {
+// If a class is not in the std namespace, we don't infer the attributes.
+class any {
+};
+} // namespace user
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===

[PATCH] D63954: Add lifetime categories attributes

[Matthias Gehre via Phabricator via cfe-commits]

mgehre updated this revision to Diff 211008.
mgehre marked 6 inline comments as done.
mgehre added a comment.

- Disallow reference and array types as DerefType
- Add example to documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63954

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaCXX/attr-gsl-owner-pointer-parsing.cpp
  clang/test/SemaCXX/attr-gsl-owner-pointer.cpp
  clang/utils/TableGen/ClangAttrEmitter.cpp

Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -303,6 +303,8 @@
 std::string getIsOmitted() const override {
   if (type == "IdentifierInfo *")
 return "!get" + getUpperName().str() + "()";
+  if (type == "TypeSourceInfo *")
+return "!get" + getUpperName().str() + "Loc()";
   if (type == "ParamIdx")
 return "!get" + getUpperName().str() + "().isValid()";
   return "false";
@@ -336,6 +338,8 @@
<< "  OS << \" \" << SA->get" << getUpperName()
<< "()->getName();\n";
   } else if (type == "TypeSourceInfo *") {
+if (isOptional())
+  OS << "if (SA->get" << getUpperName() << "Loc())";
 OS << "OS << \" \" << SA->get" << getUpperName()
<< "().getAsString();\n";
   } else if (type == "bool") {
Index: clang/test/SemaCXX/attr-gsl-owner-pointer.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-gsl-owner-pointer.cpp
@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 -ast-dump %s | \
+// RUN: FileCheck --implicit-check-not OwnerAttr --implicit-check-not PointerAttr %s
+
+class [[gsl::Owner]] OwnerMissingParameter{};
+// CHECK: CXXRecordDecl {{.*}} OwnerMissingParameter
+// CHECK: OwnerAttr
+
+class [[gsl::Pointer]] PointerMissingParameter{};
+// CHECK: CXXRecordDecl {{.*}} PointerMissingParameter
+// CHECK: PointerAttr
+
+class [[gsl::Owner()]] OwnerWithEmptyParameterList{};
+// CHECK: CXXRecordDecl {{.*}} OwnerWithEmptyParameterList
+// CHECK: OwnerAttr {{.*}}
+
+class [[gsl::Pointer()]] PointerWithEmptyParameterList{};
+// CHECK: CXXRecordDecl {{.*}} PointerWithEmptyParameterList
+// CHECK: PointerAttr {{.*}}
+
+class [[gsl::Owner(int)]] AnOwner{};
+// CHECK: CXXRecordDecl {{.*}} AnOwner
+// CHECK: OwnerAttr {{.*}} int
+
+struct S;
+class [[gsl::Pointer(S)]] APointer{};
+// CHECK: CXXRecordDecl {{.*}} APointer
+// CHECK: PointerAttr {{.*}} S
+
+class [[gsl::Owner(int)]] [[gsl::Owner(int)]] DuplicateOwner{};
+// CHECK: CXXRecordDecl {{.*}} DuplicateOwner
+// CHECK: OwnerAttr {{.*}} int
+
+class [[gsl::Pointer(int)]] [[gsl::Pointer(int)]] DuplicatePointer{};
+// CHECK: CXXRecordDecl {{.*}} DuplicatePointer
+// CHECK: PointerAttr {{.*}} int
+
+class [[gsl::Owner(int)]] AddTheSameLater{};
+// CHECK: CXXRecordDecl {{.*}} AddTheSameLater
+// CHECK: OwnerAttr {{.*}} int
+
+class [[gsl::Owner(int)]] AddTheSameLater;
+// CHECK: CXXRecordDecl {{.*}} prev {{.*}} AddTheSameLater
+// CHECK: OwnerAttr {{.*}} int
Index: clang/test/SemaCXX/attr-gsl-owner-pointer-parsing.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-gsl-owner-pointer-parsing.cpp
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int [[gsl::Owner]] i;
+// expected-error@-1 {{'Owner' attribute cannot be applied to types}}
+void [[gsl::Owner]] f();
+// expected-error@-1 {{'Owner' attribute cannot be applied to types}}
+
+[[gsl::Owner]] void f();
+// expected-warning@-1 {{'Owner' attribute only applies to classes}}
+
+struct S {
+};
+
+S [[gsl::Owner]] Instance;
+// expected-error@-1 {{'Owner' attribute cannot be applied to types}}
+
+class [[gsl::Owner(7)]] OwnerDerefNoType{};
+// expected-error@-1 {{expected a type}}
+
+class [[gsl::Pointer("int")]] PointerDerefNoType{};
+// expected-error@-1 {{expected a type}}
+
+class [[gsl::Owner(int)]] [[gsl::Pointer(int)]] BothOwnerPointer{};
+// expected-error@-1 {{'Pointer' and 'Owner' attributes are not compatible}}
+// expected-note@-2 {{conflicting attribute is here}}
+
+class [[gsl::Owner(void)]] OwnerVoidDerefType{};
+// expected-error@-1 {{'void' is an invalid argument to attribute 'Owner'}}
+class [[gsl::Pointer(void)]] PointerVoidDerefType{};
+// expected-error@-1 {{'void' is an invalid argument to attribute 'Pointer'}}
+
+class [[gsl::Pointer(int)]] AddConflictLater{};
+class [[gsl::Owner(int)]] AddConflictLater;
+// expected-error@-1 {{'Owner' and 'Pointer' attributes are not compatible}}
+// expected-note@-3 {{conflicting attribute is 

[PATCH] D63954: Add lifetime categories attributes

[Matthias Gehre via Phabricator via cfe-commits]

mgehre added a comment.

I think that I have addressed all open comments.




Comment at: clang/include/clang/Basic/AttrDocs.td:4164
+  let Content = [{
+When annotating a class ``O`` with ``[[gsl::Owner(T)]]``, then each function
+that returns cv-qualified ``T&`` or ``T*`` is assumed to return a

gribozavr wrote:
> mgehre wrote:
> > gribozavr wrote:
> > > Slightly better: "In a class annotated with ..., each function that 
> > > returns ... is assumed to ..."
> > > 
> > > However, the "is assumed" part throws me off. There's no assumption here 
> > > -- this is what this attribute means, intentionally, by design, no 
> > > guessing involved.
> > > 
> > > So how about this?
> > > 
> > > The attribute `[[gsl::Owner(T)]]` applies to structs and classes that own 
> > > an object of type `T`:
> > > 
> > > ```
> > > [[gsl::Owner(int)]] class IntOwner {
> > > private:
> > >   int value;
> > > public:
> > >   int *getInt() { return  }
> > >   long *getLong();
> > > };
> > > ```
> > > 
> > > In a class annotated like this each member function that returns `T&` or 
> > > `T*` returns a pointer/reference to the data owned by the class instance:
> > > 
> > > ```
> > > IntOwner int_owner;
> > > int *x = int_owner.getInt();
> > > // `x` points to memory owned by `int_owner`.
> > > ```
> > > 
> > > Therefore, that the lifetime of that data ends when ... For example:
> > > 
> > > ```
> > > int *x = IntOwner().getInt();
> > > // `x` points to memory owned by the temporary `IntOwner()`, which has 
> > > been destroyed now.
> > > 
> > > long *y = IntOwner().getLong();
> > > // Can't make any conclusion about validity of `y`.
> > > ```
> > > 
> > What I meant to express by "is assumed to" is that this attribute does not 
> > change what member functions do. E.g. if I had `int* f() { static int i; 
> > return  }`, then `f` would not return data owned by the class instance, 
> > even when annotating the class with `[[gsl::Owner(T)]]`. The point is that 
> > when member functions with return type `T*`/`T&` don't return data owned by 
> > the class instance, one should not add the `[[gsl::Owner(T)]]` attribute - 
> > or the analysis will produce wrong results. Here, we would warn if code 
> > uses the returned value of `f` after the class instances has been destroyed 
> > even though it would be safe.
> > 
> > There is a chicken and egg problem here. With this PR, we don't get any 
> > analysis yet, so providing examples of analysis results like "`x` points to 
> > memory owned by the temporary `IntOwner()`, which has been destroyed now." 
> > can be misleading. On the other hand, if we would just document the current 
> > meaning of the attribute, it would be nothing.
> > The implication "member function that returns `T&` or ` T*` returns a 
> > pointer/reference to the data owned by the class instance" is just one part 
> > of being an Owner. Herb's paper
> > uses this in more ways (e.g. output arguments, handling of move semantics, 
> > etc.), which we would implement incrementally together with the respective 
> > documentation update.
> > 
> > So I propose to keep the documentation basic for this PR and update the 
> > documentation with the upcoming PRs that add to the analysis. Would that be 
> > acceptable?
> > I added a note that the attribute is currently experimental and subject to 
> > change.
> > What I meant to express by "is assumed to" is that this attribute does not 
> > change what member functions do.
> 
> I don't think we should be making allowances for code that uses an attribute 
> to promise something and then does some other thing. In that model, there's 
> alignment between what code does and what attribute says.
> 
> And I mean of course the attribute does not change what the code does... that 
> must be obvious but you can mention it explicitly if you want.
> 
> > With this PR, we don't get any analysis yet
> 
> I don't think the analysis must be automatic in order to show what analysis 
> can be done. Attribute documentation needs enough examples to give people a 
> flavor of what the attribute does, and what possible benefits it can bring.
> 
> Documentation for the specific analysis that is actually done should go 
> elsewhere into Clang's documentation (if necessary).
I added an example to the documentation to show how the initial analysis will 
look like to give a feeling for what the attribute does.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4546-4547
+
+  if (ParmType->isVoidType()) {
+S.Diag(AL.getLoc(), diag::err_attribute_invalid_argument) << "'void'" << 
AL;
+return;

gribozavr wrote:
> xazax.hun wrote:
> > aaron.ballman wrote:
> > > Is `void` really the only problematic type? What about incomplete types, 
> > > for instance?
> > I think incomplete types are fine. The only information we actually need 
> > here is the name the pointee types. E.g. for a `SomeOwner` 
> > which is annotated `Owner(Incomplete)`, we will 

[PATCH] D65050: [SemaTemplate] Mark a function type as dependent when its parameter list contains pack expansion

[S. B. Tam via Phabricator via cfe-commits]

cpplearner created this revision.
cpplearner added reviewers: doug.gregor, eli.friedman, lvoufo, rsmith.
Herald added subscribers: cfe-commits, dexonsmith.
Herald added a project: clang.

Given `template using Int = int;`, the type `void(Int...)` should be 
treated as a dependent type, even though `Int` is not dependent.

This fixes https://bugs.llvm.org/show_bug.cgi?id=42654


Repository:
  rC Clang

https://reviews.llvm.org/D65050

Files:
  clang/lib/AST/Type.cpp
  clang/test/SemaTemplate/alias-templates.cpp


Index: clang/test/SemaTemplate/alias-templates.cpp
===
--- clang/test/SemaTemplate/alias-templates.cpp
+++ clang/test/SemaTemplate/alias-templates.cpp
@@ -267,3 +267,34 @@
 int z = Bar(); // expected-error {{use of template template parameter 
'Bar' requires template arguments}}
   }
 }
+
+namespace PR42654 {
+  template struct function { };
+
+  template
+  struct thing {
+void f(function) { }
+  };
+
+  template
+  struct Environment
+  {
+template
+using Integer = int;
+
+using Function = function...)>;
+using MyTuple = thing...>;
+
+void run(Function func)
+{
+  MyTuple t;
+  t.f(func);
+}
+  };
+
+  void f()
+  {
+Environment env;
+env.run({});
+  }
+}
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2912,7 +2912,7 @@
   // Fill in the trailing argument array.
   auto *argSlot = getTrailingObjects();
   for (unsigned i = 0; i != getNumParams(); ++i) {
-if (params[i]->isDependentType())
+if (params[i]->isDependentType() || params[i]->getAs())
   setDependent();
 else if (params[i]->isInstantiationDependentType())
   setInstantiationDependent();


Index: clang/test/SemaTemplate/alias-templates.cpp
===
--- clang/test/SemaTemplate/alias-templates.cpp
+++ clang/test/SemaTemplate/alias-templates.cpp
@@ -267,3 +267,34 @@
 int z = Bar(); // expected-error {{use of template template parameter 'Bar' requires template arguments}}
   }
 }
+
+namespace PR42654 {
+  template struct function { };
+
+  template
+  struct thing {
+void f(function) { }
+  };
+
+  template
+  struct Environment
+  {
+template
+using Integer = int;
+
+using Function = function...)>;
+using MyTuple = thing...>;
+
+void run(Function func)
+{
+  MyTuple t;
+  t.f(func);
+}
+  };
+
+  void f()
+  {
+Environment env;
+env.run({});
+  }
+}
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2912,7 +2912,7 @@
   // Fill in the trailing argument array.
   auto *argSlot = getTrailingObjects();
   for (unsigned i = 0; i != getNumParams(); ++i) {
-if (params[i]->isDependentType())
+if (params[i]->isDependentType() || params[i]->getAs())
   setDependent();
 else if (params[i]->isInstantiationDependentType())
   setInstantiationDependent();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63889: Check possible warnings on global initializers for reachability

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

rsmith added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:352
   SetParamDefaultArgument(Param, DefaultArg, EqualLoc);
+  CurContext->removeDecl(Param);
+  CurContext = Cur;

We may need to delay the diagnostics here until the default argument is *used*: 
if a default argument references a template instantiation, the instantiation is 
not performed until that point, which may mean that our semantic checking can't 
complete correctly until use.



Comment at: clang/lib/Sema/SemaExpr.cpp:16694
+// initializer and check whether the context in question is reachable.
+if (auto *VD = dyn_cast_or_null(CurContext->getLastDecl())) {
+  if (VD->getDefinition()) {

It's not correct to assume that the last declaration in the context is the 
right one to be considering here. Instead, you should track whether you're in a 
variable initializer (and for what) in `Sema`. Examples:

```
int x = ([]{}(), x); // in C++; last decl is the lambda
```

```
int y = (struct Q { int n; }){y}; // in C; last decl is the struct
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63889



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

[PATCH] D64914: Implement P1771

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

rsmith added a comment.

Please update cxx_status.html to mark P1771R1 as implemented in SVN.




Comment at: clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp:87-88
 // expected-warning@28 {{use of the 'nodiscard' attribute is a C++17 
extension}}
+// expected-warning@66 {{use of the 'nodiscard' attribute is a C++17 
extension}}
+// expected-warning@71 {{use of the 'nodiscard' attribute is a C++17 
extension}}
 #endif

aaron.ballman wrote:
> erichkeane wrote:
> > aaron.ballman wrote:
> > > Is Core treating this paper as a DR? I don't have a strong opinion here, 
> > > but for the nodiscard with a message version, I made it a C++2a 
> > > extension. I don't have a strong opinion, but I sort of prefer doing 
> > > whatever Core decides.
> > I am unfamiliar with what Core is treating it as, but my understanding is 
> > that EWG encouraged implementers to treat it as such.  
> We expose the attribute in all its glory in all language modes, so these 
> changes already do what we want in effect. The only real question is whether 
> we want to claim it's a C++17 extension or a C++2a extension. If a user turns 
> on extension warnings, we should probably tell them when the feature was 
> added, which is C++2a. It would be a bit weird to claim this is a C++17 when 
> the feature test for it is `__has_attribute(nodiscard) == 201907L` (due to 
> the normative wording changes).
> 
> But if Core moves it as a DR, then C++17 is fine, though I suppose SD-6 would 
> need to make clear what is required for each given attribute feature test 
> value to give us the answer.
We moved this change as a DR, so this feature should be treated as if it were 
always part of the spec.


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

https://reviews.llvm.org/D64914



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

Re: r366626 - Implement P1301R4, which allows specifying an optional message on the [[nodiscard]] attribute.

[Richard Smith via cfe-commits]

On Sat, 20 Jul 2019 at 09:55, Aaron Ballman via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: aaronballman
> Date: Sat Jul 20 00:56:34 2019
> New Revision: 366626
>
> URL: http://llvm.org/viewvc/llvm-project?rev=366626=rev
> Log:
> Implement P1301R4, which allows specifying an optional message on the
> [[nodiscard]] attribute.
>
> This also bumps the attribute feature test value and introduces the notion
> of a C++2a extension warning.
>
> Modified:
> cfe/trunk/include/clang/Basic/Attr.td
> cfe/trunk/include/clang/Basic/AttrDocs.td
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> cfe/trunk/lib/Sema/SemaStmt.cpp
> cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p1.cpp
> cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp
> cfe/trunk/test/Preprocessor/has_attribute.cpp
> cfe/trunk/test/Sema/c2x-nodiscard.c
> cfe/trunk/test/SemaCXX/cxx11-attr-print.cpp
>
> Modified: cfe/trunk/include/clang/Basic/Attr.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=366626=366625=366626=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/Attr.td (original)
> +++ cfe/trunk/include/clang/Basic/Attr.td Sat Jul 20 00:56:34 2019
> @@ -2335,10 +2335,11 @@ def WarnUnused : InheritableAttr {
>  }
>
>  def WarnUnusedResult : InheritableAttr {
> -  let Spellings = [CXX11<"", "nodiscard", 201603>, C2x<"", "nodiscard">,
> +  let Spellings = [CXX11<"", "nodiscard", 201907>, C2x<"", "nodiscard">,
>

We shouldn't bump the version until we also implement P1771R1, as this
number indicates that both papers are implemented.


> CXX11<"clang", "warn_unused_result">,
> GCC<"warn_unused_result">];
>let Subjects = SubjectList<[ObjCMethod, Enum, Record, FunctionLike]>;
> +  let Args = [StringArgument<"Message", 1>];
>let Documentation = [WarnUnusedResultsDocs];
>  }
>
>
> Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=366626=366625=366626=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
> +++ cfe/trunk/include/clang/Basic/AttrDocs.td Sat Jul 20 00:56:34 2019
> @@ -1482,6 +1482,13 @@ generated when a function or its return
>  potentially-evaluated discarded-value expression that is not explicitly
> cast to
>  `void`.
>
> +A string literal may optionally be provided to the attribute, which will
> be
> +reproduced in any resulting diagnostics. Redeclarations using different
> forms
> +of the attribute (with or without the string literal or with different
> string
> +literal contents) are allowed. If there are redeclarations of the entity
> with
> +differing string literals, it is unspecified which one will be used by
> Clang
> +in any resulting diagnostics.
> +
>  .. code-block: c++
>struct [[nodiscard]] error_info { /*...*/ };
>error_info enable_missile_safety_mode();
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=366626=366625=366626=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Sat Jul 20
> 00:56:34 2019
> @@ -7436,6 +7436,9 @@ def warn_side_effects_typeid : Warning<
>  def warn_unused_result : Warning<
>"ignoring return value of function declared with %0 attribute">,
>InGroup;
> +def warn_unused_result_msg : Warning<
> +  "ignoring return value of function declared with %0 attribute: %1">,
> +  InGroup;
>  def warn_unused_volatile : Warning<
>"expression result unused; assign into a variable to force a volatile
> load">,
>InGroup>;
> @@ -7444,6 +7447,8 @@ def ext_cxx14_attr : Extension<
>"use of the %0 attribute is a C++14 extension">, InGroup;
>  def ext_cxx17_attr : Extension<
>"use of the %0 attribute is a C++17 extension">, InGroup;
> +def ext_cxx2a_attr : Extension<
> +  "use of the %0 attribute is a C++2a extension">, InGroup;
>
>  def warn_unused_comparison : Warning<
>"%select{equality|inequality|relational|three-way}0 comparison result
> unused">,
>
> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=366626=366625=366626=diff
>
> ==
> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Sat Jul 20 00:56:34 2019
> @@ -2841,14 +2841,30 @@ static void handleWarnUnusedResult(Sema
>return;
>  }
>
> -  // If this is spelled as the standard C++17 

[PATCH] D59555: [analyzer] Add yaml parser to GenericTaintChecker

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

Szelethus accepted this revision.
Szelethus added a comment.

LGTM, thanks! Please mark inlines as done as you fix them. If you like the file 
description I proposed, I'm happy to commit this on your behalf (or you might 
as well apply for a commit access, since you have a track record of high 
quality patches already).




Comment at: lib/StaticAnalyzer/Checkers/Yaml.h:9
+//
+// This file contains a simple interface allow to open and parse yaml files.
+//

Szelethus wrote:
> Is this actually the reason why we have this file? We already have a YAML 
> parser in LLVM, what's does this file do that the "default" parser doesn't?
I would appreciate some comments, it still isn't immediately obvious why we 
need yet another yaml file. How about

"This file defines convenience functions for handling YAML configuration files 
for checkers/packages".


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

https://reviews.llvm.org/D59555



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