Re: [libcxxabi] r308470 - Drop 'svn' suffix from version number.

[Dimitry Andric via cfe-commits]

On 19 Jul 2017, at 16:04, Hans Wennborg via cfe-commits 
 wrote:
> 
> Author: hans
> Date: Wed Jul 19 07:04:19 2017
> New Revision: 308470
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=308470=rev
> Log:
> Drop 'svn' suffix from version number.

[Replying to this commit, since I don't have r308469 (which does the same for 
llvm itself) in my mailboxes.]

Note this approach isn't effective anymore after Rafael's
https://reviews.llvm.org/rL306858, which turns on the LLVM_APPEND_VC_REV
option by default.

The handling of that option will overwrite the PACKAGE_VERSION value set
earlier in CMakeLists.txt, with the value returned from
add_version_info_from_vcs().  When using Subversion, that will always be
of the form X.Y.Zsvn-rNN.

I noticed this when preparing the 5.0.0 final import into FreeBSD, where
for example "opt -version" outputs:

LLVM (http://llvm.org/):
  LLVM version 5.0.0svn-r312559
  Optimized build with assertions.
  Default target: x86_64-unknown-freebsd12.0
  Host CPU: ivybridge

-Dimitry



signature.asc
Description: Message signed with OpenPGP
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37308: Interface class with uuid base record

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam updated this revision to Diff 114497.
zahiraam added a comment.

Erich,

Addressed your comments.
Thanks.


https://reviews.llvm.org/D37308

Files:
  lib/Sema/SemaDeclCXX.cpp
  test/SemaCXX/ms-uuid.cpp


Index: test/SemaCXX/ms-uuid.cpp
===
--- test/SemaCXX/ms-uuid.cpp
+++ test/SemaCXX/ms-uuid.cpp
@@ -93,3 +93,32 @@
 [uuid("00A0---C000-0049"),
  uuid("00A0---C000-0049")] class C10;
 }
+
+struct __declspec(uuid("---C000-0046")) IUnknown {};
+struct IPropertyPageBase : public IUnknown {};
+struct IPropertyPage : public IPropertyPageBase {};
+__interface ISfFileIOPropertyPage : public IPropertyPage {};
+
+
+__interface ISfFileIOPropertyPage1 : public IUnknown {};
+
+struct __declspec(uuid("---C000-0045")) IUnknown1 {};
+__interface ISfFileIOPropertyPage2 : public IUnknown1 {};  // expected-error 
{{interface type cannot inherit from}}
+
+struct __declspec(uuid("---C000-0046")) IUnknown2 {};
+struct IPropertyPage2 : public IUnknown2 {};
+__interface ISfFileIOPropertyPage3 : public IPropertyPage2 {}; // 
expected-error {{interface type cannot inherit from}}
+
+struct IPropertyPage3 : public IUnknown {};
+__interface ISfFileIOPropertyPage4 : public IPropertyPage3 {};
+
+__interface __declspec(dllimport) ISfFileIOPropertyPage33 : public IUnknown {};
+
+struct __declspec(uuid("---C000-0046")) IUnknown4 {};
+__interface ISfFileIOPropertyPage5 : public IUnknown4 {}; // expected-error 
{{interface type cannot inherit from}}
+
+class __declspec(uuid("---C000-0046")) IUnknown5 {};
+__interface ISfFileIOPropertyPage6 : public IUnknown5 {}; // expected-error 
{{interface type cannot inherit from}}
+
+struct __declspec(dllexport) IUnknown6{};
+__interface foo : public IUnknown6{}; // expected-error {{interface type 
cannot inherit from}}
Index: lib/Sema/SemaDeclCXX.cpp
===
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -2373,6 +2373,33 @@
   return true;
 }
 
+/// \brief Tests if the __interface base is public.
+static bool IsDeclPublicInterface(const CXXRecordDecl *RD,
+  AccessSpecifier spec) {
+  return RD->isInterface() && spec == AS_public;
+}
+
+/// \brief Test if record is an uuid Unknown.
+/// This is an MS SDK specific type that has a special
+/// behavior in the CL compiler.
+static bool IsIUnknownType(const CXXRecordDecl *RD) {
+  auto *Uuid = RD->getAttr();
+
+  return RD->isStruct() && RD->getName() == "IUnknown" && RD->isEmpty() &&
+ Uuid && Uuid->getGuid() =="---C000-0046" &&
+ dyn_cast(RD->getDeclContext());
+}
+
+/// \brief Test if RD or its inhetited bases is an IUnknow type.
+static bool IsOrInheritsFromIUnknown(const CXXRecordDecl *RD) {
+  const CXXRecordDecl *Base =
+  RD->getNumBases() ? RD->bases_begin()->getType()->getAsCXXRecordDecl()
+: nullptr;
+
+  return IsIUnknownType(RD) ||
+ Base &&  (IsIUnknownType(Base) || IsOrInheritsFromIUnknown(Base));
+}
+
 /// Use small set to collect indirect bases.  As this is only used
 /// locally, there's no need to abstract the small size parameter.
 typedef llvm::SmallPtrSet IndirectBaseSet;
@@ -2450,10 +2477,10 @@
   if (const RecordType *Record = NewBaseType->getAs()) {
 const CXXRecordDecl *RD = cast(Record->getDecl());
 if (Class->isInterface() &&
-  (!RD->isInterface() ||
-   KnownBase->getAccessSpecifier() != AS_public)) {
-  // The Microsoft extension __interface does not permit bases that
-  // are not themselves public interfaces.
+// The Microsoft extension __interface does not permit bases that
+// are not themselves public interfaces.
+!IsDeclPublicInterface(RD, KnownBase->getAccessSpecifier()) &&
+!IsOrInheritsFromIUnknown(RD)) {
   Diag(KnownBase->getLocStart(), diag::err_invalid_base_in_interface)
 << getRecordDiagFromTagKind(RD->getTagKind()) << RD->getName()
 << RD->getSourceRange();


Index: test/SemaCXX/ms-uuid.cpp
===
--- test/SemaCXX/ms-uuid.cpp
+++ test/SemaCXX/ms-uuid.cpp
@@ -93,3 +93,32 @@
 [uuid("00A0---C000-0049"),
  uuid("00A0---C000-0049")] class C10;
 }
+
+struct __declspec(uuid("---C000-0046")) IUnknown {};
+struct IPropertyPageBase : public IUnknown {};
+struct IPropertyPage : public IPropertyPageBase {};
+__interface ISfFileIOPropertyPage : public IPropertyPage {};
+
+
+__interface ISfFileIOPropertyPage1 : public IUnknown {};
+
+struct __declspec(uuid("---C000-0045")) IUnknown1 {};
+__interface ISfFileIOPropertyPage2 

r312870 - clang fix for LLVM API change: isKnownNonNull -> isKnownNonZero

[Nuno Lopes via cfe-commits]

Author: nlopes
Date: Sat Sep  9 11:25:36 2017
New Revision: 312870

URL: http://llvm.org/viewvc/llvm-project?rev=312870=rev
Log:
clang fix for LLVM API change: isKnownNonNull -> isKnownNonZero

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

Modified:
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/test/CodeGenCXX/microsoft-abi-dynamic-cast.cpp

Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=312870=312869=312870=diff
==
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Sat Sep  9 11:25:36 2017
@@ -3055,7 +3055,8 @@ static void emitWriteback(CodeGenFunctio
 
   // If the argument wasn't provably non-null, we need to null check
   // before doing the store.
-  bool provablyNonNull = llvm::isKnownNonNull(srcAddr.getPointer());
+  bool provablyNonNull = llvm::isKnownNonZero(srcAddr.getPointer(),
+  CGF.CGM.getDataLayout());
   if (!provablyNonNull) {
 llvm::BasicBlock *writebackBB = CGF.createBasicBlock("icr.writeback");
 contBB = CGF.createBasicBlock("icr.done");
@@ -3195,7 +3196,8 @@ static void emitWritebackArg(CodeGenFunc
   // If the address is *not* known to be non-null, we need to switch.
   llvm::Value *finalArgument;
 
-  bool provablyNonNull = llvm::isKnownNonNull(srcAddr.getPointer());
+  bool provablyNonNull = llvm::isKnownNonZero(srcAddr.getPointer(),
+  CGF.CGM.getDataLayout());
   if (provablyNonNull) {
 finalArgument = temp.getPointer();
   } else {

Modified: cfe/trunk/test/CodeGenCXX/microsoft-abi-dynamic-cast.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/microsoft-abi-dynamic-cast.cpp?rev=312870=312869=312870=diff
==
--- cfe/trunk/test/CodeGenCXX/microsoft-abi-dynamic-cast.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/microsoft-abi-dynamic-cast.cpp Sat Sep  9 
11:25:36 2017
@@ -78,7 +78,7 @@ T* test6(B* x) { return dynamic_cast
 // CHECK-NEXT:   [[VBOFFS:%.*]] = load i32, i32* [[VBOFFP]], align 4
 // CHECK-NEXT:   [[DELTA:%.*]] = add nsw i32 [[VBOFFS]], 4
 // CHECK-NEXT:   [[ADJ:%.*]] = getelementptr inbounds i8, i8* [[CAST]], i32 
[[DELTA]]
-// CHECK-NEXT:   [[CALL:%.*]] = tail call i8* @__RTDynamicCast(i8* [[ADJ]], 
i32 [[DELTA]], i8* {{.*}}bitcast (%rtti.TypeDescriptor7* @"\01??_R0?AUB@@@8" to 
i8*), i8* {{.*}}bitcast (%rtti.TypeDescriptor7* @"\01??_R0?AUT@@@8" to i8*), 
i32 0)
+// CHECK-NEXT:   [[CALL:%.*]] = tail call i8* @__RTDynamicCast(i8* nonnull 
[[ADJ]], i32 [[DELTA]], i8* {{.*}}bitcast (%rtti.TypeDescriptor7* 
@"\01??_R0?AUB@@@8" to i8*), i8* {{.*}}bitcast (%rtti.TypeDescriptor7* 
@"\01??_R0?AUT@@@8" to i8*), i32 0)
 // CHECK-NEXT:   [[RES:%.*]] = bitcast i8* [[CALL]] to %struct.T*
 // CHECK-NEXT:   br label
 // CHECK:[[RET:%.*]] = phi %struct.T*
@@ -117,7 +117,7 @@ void* test9(B* x) { return dynamic_cast<
 // CHECK-NEXT:   [[VBOFFS:%.*]] = load i32, i32* [[VBOFFP]], align 4
 // CHECK-NEXT:   [[DELTA:%.*]] = add nsw i32 [[VBOFFS]], 4
 // CHECK-NEXT:   [[ADJ:%.*]] = getelementptr inbounds i8, i8* [[CAST]], i32 
[[DELTA]]
-// CHECK-NEXT:   [[CALL:%.*]] = tail call i8* @__RTCastToVoid(i8* [[ADJ]])
+// CHECK-NEXT:   [[CALL:%.*]] = tail call i8* @__RTCastToVoid(i8* nonnull 
[[ADJ]])
 // CHECK-NEXT:   br label
 // CHECK:[[RET:%.*]] = phi i8*
 // CHECK-NEXT:   ret i8* [[RET]]


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

r312865 - [Basic] Update CMakeLists.txt to handle repo

[MinSeong Kim via cfe-commits]

Author: minseongkim
Date: Sat Sep  9 07:18:53 2017
New Revision: 312865

URL: http://llvm.org/viewvc/llvm-project?rev=312865=rev
Log:
[Basic] Update CMakeLists.txt to handle repo

Summary:
The find_first_existing_file and find_first_existing_vc_file macros in
lib/Basic/CMakeLists.txt are removed. The macros are also defined in
{LLVM}/cmake/modules/AddLLVM.cmake for the same purpose. This change
serves the following 2 objectives:

1. To remove the redundant code in clang to use the same
   macros in llvm,
2. The macros in AddLLVM.cmake can also handle repo for
   displaying correct version information.

Reviewers: jordan_rose, cfe-commits, modocache, hintonda

Reviewed By: hintonda

Subscribers: mgorny

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

Modified:
cfe/trunk/lib/Basic/CMakeLists.txt

Modified: cfe/trunk/lib/Basic/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/CMakeLists.txt?rev=312865=312864=312865=diff
==
--- cfe/trunk/lib/Basic/CMakeLists.txt (original)
+++ cfe/trunk/lib/Basic/CMakeLists.txt Sat Sep  9 07:18:53 2017
@@ -4,39 +4,6 @@ set(LLVM_LINK_COMPONENTS
   Support
   )
 
-# Figure out if we can track VC revisions.
-function(find_first_existing_file out_var)
-  foreach(file ${ARGN})
-if(EXISTS "${file}")
-  set(${out_var} "${file}" PARENT_SCOPE)
-  return()
-endif()
-  endforeach()
-endfunction()
-
-macro(find_first_existing_vc_file out_var path)
-  set(git_path "${path}/.git")
-
-  # Normally '.git' is a directory that contains a 'logs/HEAD' file that
-  # is updated as modifications are made to the repository. In case the
-  # repository is a Git submodule, '.git' is a file that contains text that
-  # indicates where the repository's Git directory exists.
-  if (EXISTS "${git_path}" AND NOT IS_DIRECTORY "${git_path}")
-FILE(READ "${git_path}" file_contents)
-if("${file_contents}" MATCHES "^gitdir: ([^\n]+)")
-  # '.git' is indeed a link to the submodule's Git directory.
-  # Use the path to that Git directory.
-  set(git_path "${path}/${CMAKE_MATCH_1}")
-endif()
-  endif()
-
-  find_first_existing_file(${out_var}
-"${git_path}/logs/HEAD"  # Git or Git submodule
-"${path}/.svn/wc.db" # SVN 1.7
-"${path}/.svn/entries"   # SVN 1.6
-)
-endmacro()
-
 find_first_existing_vc_file(llvm_vc "${LLVM_MAIN_SRC_DIR}")
 find_first_existing_vc_file(clang_vc "${CLANG_SOURCE_DIR}")
 


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

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

[Blitz Rakete via Phabricator via cfe-commits]

Rakete added a comment.

> Is that a problem?

I don't think so, because `shouldVisitImplicitCode()` doesn't mean that it only 
visits implicit code, it's not `onlyVisitImplicitCode()` :) In fact, I would be 
surprised if the template parameters were only visited once.




Comment at: lib/AST/ExprCXX.cpp:21
 #include "clang/Basic/IdentifierTable.h"
+#include "llvm/ADT/STLExtras.h"
 using namespace clang;

You don't need this header.



Comment at: lib/AST/ExprCXX.cpp:1010
+  return std::count_if(List->begin(), List->end(),
+   [](const NamedDecl *D) { return !D->isImplicit(); });
 }

You could store the lambda in a variable instead of having two times the same 
exact lambda expression.



Comment at: lib/Sema/SemaLambda.cpp:840
+  "template param scope");
+KnownDependent = TemplateParamScope->getParent()
+   ->getTemplateParamParent() != nullptr;

I think you should add an `assert` to verify that `getParent()` doesn't return 
`nullptr`, just to be safe from UB.



Comment at: unittests/AST/StmtPrinterTest.cpp:126
+ const T , StringRef ExpectedPrinted) {
+  std::vector Args {
+"-std=c++98",

LLVM style guide says that if you are using a braced-init-list to initialize an 
object, you have to use an `=`.


https://reviews.llvm.org/D36527



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

[PATCH] D35533: [Basic] Update CMakeLists.txt to handle Repo with git

[MinSeong KIM via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL312865: [Basic] Update CMakeLists.txt to handle repo 
(authored by MinSeongKIM).

Changed prior to commit:
  https://reviews.llvm.org/D35533?vs=114160=114488#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D35533

Files:
  cfe/trunk/lib/Basic/CMakeLists.txt


Index: cfe/trunk/lib/Basic/CMakeLists.txt
===
--- cfe/trunk/lib/Basic/CMakeLists.txt
+++ cfe/trunk/lib/Basic/CMakeLists.txt
@@ -4,39 +4,6 @@
   Support
   )
 
-# Figure out if we can track VC revisions.
-function(find_first_existing_file out_var)
-  foreach(file ${ARGN})
-if(EXISTS "${file}")
-  set(${out_var} "${file}" PARENT_SCOPE)
-  return()
-endif()
-  endforeach()
-endfunction()
-
-macro(find_first_existing_vc_file out_var path)
-  set(git_path "${path}/.git")
-
-  # Normally '.git' is a directory that contains a 'logs/HEAD' file that
-  # is updated as modifications are made to the repository. In case the
-  # repository is a Git submodule, '.git' is a file that contains text that
-  # indicates where the repository's Git directory exists.
-  if (EXISTS "${git_path}" AND NOT IS_DIRECTORY "${git_path}")
-FILE(READ "${git_path}" file_contents)
-if("${file_contents}" MATCHES "^gitdir: ([^\n]+)")
-  # '.git' is indeed a link to the submodule's Git directory.
-  # Use the path to that Git directory.
-  set(git_path "${path}/${CMAKE_MATCH_1}")
-endif()
-  endif()
-
-  find_first_existing_file(${out_var}
-"${git_path}/logs/HEAD"  # Git or Git submodule
-"${path}/.svn/wc.db" # SVN 1.7
-"${path}/.svn/entries"   # SVN 1.6
-)
-endmacro()
-
 find_first_existing_vc_file(llvm_vc "${LLVM_MAIN_SRC_DIR}")
 find_first_existing_vc_file(clang_vc "${CLANG_SOURCE_DIR}")
 


Index: cfe/trunk/lib/Basic/CMakeLists.txt
===
--- cfe/trunk/lib/Basic/CMakeLists.txt
+++ cfe/trunk/lib/Basic/CMakeLists.txt
@@ -4,39 +4,6 @@
   Support
   )
 
-# Figure out if we can track VC revisions.
-function(find_first_existing_file out_var)
-  foreach(file ${ARGN})
-if(EXISTS "${file}")
-  set(${out_var} "${file}" PARENT_SCOPE)
-  return()
-endif()
-  endforeach()
-endfunction()
-
-macro(find_first_existing_vc_file out_var path)
-  set(git_path "${path}/.git")
-
-  # Normally '.git' is a directory that contains a 'logs/HEAD' file that
-  # is updated as modifications are made to the repository. In case the
-  # repository is a Git submodule, '.git' is a file that contains text that
-  # indicates where the repository's Git directory exists.
-  if (EXISTS "${git_path}" AND NOT IS_DIRECTORY "${git_path}")
-FILE(READ "${git_path}" file_contents)
-if("${file_contents}" MATCHES "^gitdir: ([^\n]+)")
-  # '.git' is indeed a link to the submodule's Git directory.
-  # Use the path to that Git directory.
-  set(git_path "${path}/${CMAKE_MATCH_1}")
-endif()
-  endif()
-
-  find_first_existing_file(${out_var}
-"${git_path}/logs/HEAD"  # Git or Git submodule
-"${path}/.svn/wc.db" # SVN 1.7
-"${path}/.svn/entries"   # SVN 1.6
-)
-endmacro()
-
 find_first_existing_vc_file(llvm_vc "${LLVM_MAIN_SRC_DIR}")
 find_first_existing_vc_file(clang_vc "${CLANG_SOURCE_DIR}")
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36354: [clang-tidy] Implement type-based check for `gsl::owner`

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

- Adressed some review comments.
- Added testcases, where the `owner<>` annotation might be hidden by a 
`typedef` or `using`, like int `using heap_int = gsl::owner;`

The check is currently not able to resolve a `typedef` to `owner<>` correctly, 
and my knowledge in clang is to limited to find exact reason and/or a 
workaround.
I tried to match the type with `hasCanonicalType`, but i guess that will remove 
the `owner<>` alias and therefore be not an option.
The basic testcase for `typedef` should stay in the code with the FIXME, since 
there is probably a way around.

I am thinking of a matcher like `IsTypedefToOwner`, that looks through one 
typedef after another recursively until it finds an `owner<>`, but later and 
probably with a different solution.

The typedef issue is in the following lines, just as shortcut since the patch 
is quite big:
testcase: 200-225

The issue is mentioned in the docs as well.




Comment at: test/clang-tidy/cppcoreguidelines-owning-memory.cpp:39
+  return new int(42);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: returning a 'gsl::owner<>' from a 
function but not declaring it; return type is 'int *'
+}

aaron.ballman wrote:
> JonasToth wrote:
> > aaron.ballman wrote:
> > > This diagnostic confuses me -- there's no gsl::owner<> involved anywhere; 
> > > am I missing something?
> > `owner<>` is not involved, but the guidelines say, that `new` must be 
> > assigned to an owner.
> > 
> > This is in line with the resource semantics. Everything that creates an 
> > resource, that must be released (no RAII available) shall be annotated.
> > 
> > The diagnostic is bad, though.
> > 
> > `Returning a newly created resource from function 'functionname', without 
> > declaring it as 'gsl::owner<>'; type is '...'`
> Okay, that makes more sense to me. I don't think the name of the function 
> helps all that much in the diagnostic, however. What about:
> 
> `"returning a newly created resource of type %0 from a function whose return 
> type is not 'gsl::owner<>'"`
There is a minor issue with the diagnostic in general, since it is emitted for 
values of type `gsl::owner<>` and values that are known to be an owner like 
`new int(42)`.

There is no easy way to distinguish between a recognized resource or an 
annotated resource, without complicating the matchers (what i dont want, since 
there is already a lot happening).

Mixing both cases in the diagnostic would help, but go in the direction of 
`recognized resource`, that was decided against earlier.

Would the following modification be acceptable as well?
`returning a newly created resource of type %0 or value of type 'gsl::owner<>' 
from a function whose return type is not 'gsl::owner<>'`
or
`returning a newly created resource of type %0 or value of type 'gsl::owner<>' 
without annotating the return type of the function as 'gsl::owner<>'`.

This general problem holds true for other cases, since i want to match for 
`IsConsideredOwner`, which wraps cases like `new`, functions returning 
`owner<>` and variables of type `owner<>`. 
I want to expand this further to functions that should return `owner<>` but 
can't, like `malloc`.
Splitting up the matchers instead of using `IsConsideredOwner` would be a 
burden including a lot of code duplication.


https://reviews.llvm.org/D36354



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

[PATCH] D36354: [clang-tidy] Implement type-based check for `gsl::owner`

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 114483.
JonasToth marked 4 inline comments as done.
JonasToth added a comment.

- Merge branch 'master'
- mention current problems with typedefs of owner<> and adjust test cases


https://reviews.llvm.org/D36354

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp
  clang-tidy/cppcoreguidelines/OwningMemoryCheck.h
  clang-tidy/utils/Matchers.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-owning-memory.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-owning-memory.cpp

Index: test/clang-tidy/cppcoreguidelines-owning-memory.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-owning-memory.cpp
@@ -0,0 +1,382 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-owning-memory %t
+
+namespace gsl {
+template 
+using owner = T;
+} // namespace gsl
+
+template 
+class unique_ptr {
+public:
+  unique_ptr(gsl::owner resource) : memory(resource) {}
+  unique_ptr(const unique_ptr &) = default;
+
+  ~unique_ptr() { delete memory; }
+
+private:
+  gsl::owner memory;
+};
+
+void takes_owner(gsl::owner owned_int) {
+}
+
+void takes_pointer(int *unowned_int) {
+}
+
+void takes_owner_and_more(int some_int, gsl::owner owned_int, float f) {
+}
+
+template 
+void takes_templated_owner(gsl::owner owned_T) {
+}
+
+gsl::owner returns_owner1() { return gsl::owner(new int(42)); } // Ok
+gsl::owner returns_owner2() { return new int(42); }// Ok
+
+int *returns_no_owner1() { return nullptr; }
+int *returns_no_owner2() {
+  return new int(42);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: returning a 'gsl::owner<>' from a function but not declaring it; return type is 'int *'
+}
+int *returns_no_owner3() {
+  int *should_be_owner = new int(42);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: initializing non-owner 'int *' with a newly created 'gsl::owner<>'
+  return should_be_owner;
+}
+int *returns_no_owner4() {
+  gsl::owner owner = new int(42);
+  return owner;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: returning a 'gsl::owner<>' from a function but not declaring it; return type is 'int *'
+}
+
+unique_ptr returns_no_owner5() {
+  return unique_ptr(new int(42)); // Ok
+}
+
+/// FIXME: CSA finds it, but the report is misleading. Ownersemantics can catch this
+/// by flow analysis similar to misc-use-after-move.
+void csa_not_finding_leak() {
+  gsl::owner o1 = new int(42); // Ok
+
+  gsl::owner o2 = o1; // Ok
+  o2 = new int(45);  // conceptual leak, the memory from o1 is now leaked, since its considered moved in the guidelines
+
+  delete o2;
+  // actual leak occurs here, its found, but mixed
+  delete o1;
+}
+
+void test_assignment_and_initialization() {
+  int stack_int1 = 15;
+  int stack_int2;
+
+  gsl::owner owned_int1 = _int1; // BAD
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expected initialization with value of type 'gsl::owner<>'; got 'int *'
+
+  gsl::owner owned_int2;
+  owned_int2 = _int2; // BAD since no owner, bad since uninitialized
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expected assignment source to be of type 'gsl::owner<>'; got 'int *'
+
+  gsl::owner owned_int3 = new int(42); // Good
+  owned_int3 = nullptr;   // Good
+
+  gsl::owner owned_int4(nullptr); // Ok
+  owned_int4 = new int(42);  // Good
+
+  gsl::owner owned_int5 = owned_int3; // Good
+
+  gsl::owner owned_int6{nullptr}; // Ok
+  owned_int6 = owned_int4;   // Good
+
+  // FIXME:, flow analysis for the case of reassignment. Value must be released before
+  owned_int6 = owned_int3; // BAD, because reassignment without resource release
+
+  auto owned_int7 = returns_owner1(); // Bad, since typededuction eliminates the owner wrapper
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: initializing non-owner 'int *' with a newly created 'gsl::owner<>'
+  // CHECK-MESSAGES: [[@LINE-2]]:3: note: type deduction did not result in an owner
+
+  const auto owned_int8 = returns_owner2(); // Bad, since typededuction eliminates the owner wrapper
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: initializing non-owner 'int *const' with a newly created 'gsl::owner<>'
+  // CHECK-MESSAGES: [[@LINE-2]]:3: note: type deduction did not result in an owner
+
+  gsl::owner owned_int9 = returns_owner1(); // Ok
+  int *unowned_int3 = returns_owner1();// Bad
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: initializing non-owner 'int *' with a newly created 'gsl::owner<>'
+
+  gsl::owner owned_int10;
+  owned_int10 = returns_owner1(); // Ok
+
+  int *unowned_int4;
+  unowned_int4 = returns_owner1(); // Bad
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: assigning newly created 'gsl::owner<>' to non-owner 'int *'
+
+  gsl::owner owned_int11 = returns_no_owner1(); // Bad since no owner
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: 

Re: r312633 - [AST] Traverse CXXOperatorCallExpr in LexicallyOrderedRecursiveASTVisitor

[Johannes Altmanninger via cfe-commits]

Richard Smith  writes:

> I am extremely uncomfortable about the direction this patch series is going.
>
> We have had two different RecursiveASTVisitors before (RecursiveASTVisitor
> and DataRecursiveASTVisitor), and it was a maintenance nightmare:
> frequently changes would be made to one of them and missed in the other
> one, resulting in hard to track down bugs, as well as redundant development
> effort making changes to both.
>
> Back when this was simply deriving from RecursiveASTVisitor and not
> changing much, LexicallyOrderedRecursiveASTVisitor seemed like it wouldn't
> suffer from the same problem, but it's becoming increasingly clear that
> that simply isn't going to be the case long-term. And worse, there seems to
> be absolutely no purpose in having two different visitors here -- the
> existing users of RecursiveASTVisitor generally don't care about visitation
> order.
>
> Also, we can play the "what if two people did this" game -- adding RAV
> functionality in derived classes doesn't compose well. (If post-order
> traversal were a derived class, you couldn't request a lexically-ordered
> post-order traversal, and so on.)
>
> Therefore, I'd like to suggest that you stop making changes to
> LexicallyOrderedRecursiveASTVisitor and instead start to fold its
> functionality back into RecursiveASTVisitor, as follows:
>
>  * in cases where there is no reason for RAV to visit in non-lexical order,
> change it to visit in lexical order
>  * if there are any cases where there *is* a reason for non-lexical
> visitation, add a bool function to it so the derived class can request
> lexical / non-lexical order (like the existing shouldTraversePostOrder /
> shouldVisitImplicitCode / ... functions).

Ok, makes sense.

+Alex so you are aware.

I have created two revisions to move my changes to RecursiveASTVisitor,
this should not break anything. I left the tests in
LexicallyOrderedRecursiveASTVisitorTest for now because it saves some
code duplication, but let's see, if we merge all the changes into
RecursiveASTVisitor, then the tests will naturally follow.

https://reviews.llvm.org/D37662
https://reviews.llvm.org/D37663

>
> On 6 September 2017 at 06:12, Johannes Altmanninger via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: krobelus
>> Date: Wed Sep  6 06:12:11 2017
>> New Revision: 312633
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=312633=rev
>> Log:
>> [AST] Traverse CXXOperatorCallExpr in LexicallyOrderedRecursiveASTVisitor
>>
>> Summary:
>> This affects overloaded operators, which are represented by a
>> CXXOperatorCallExpr whose first child is always a DeclRefExpr referring to
>> the
>> operator. For infix, postfix and call operators we want the first argument
>> to be traversed before the operator.
>>
>> Reviewers: arphaman
>>
>> Subscribers: klimek, cfe-commits
>>
>> Differential Revision: https://reviews.llvm.org/D37200
>>
>> Modified:
>> cfe/trunk/include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h
>> cfe/trunk/include/clang/AST/RecursiveASTVisitor.h
>> cfe/trunk/unittests/Tooling/LexicallyOrderedRecursiveASTVi
>> sitorTest.cpp
>>
>> Modified: cfe/trunk/include/clang/AST/LexicallyOrderedRecursiveASTVi
>> sitor.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/
>> LexicallyOrderedRecursiveASTVisitor.h?rev=312633=312632&
>> r2=312633=diff
>> 
>> ==
>> --- cfe/trunk/include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h
>> (original)
>> +++ cfe/trunk/include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h Wed
>> Sep  6 06:12:11 2017
>> @@ -113,6 +113,33 @@ public:
>>
>>bool shouldTraverseTemplateArgumentsBeforeDecl() const { return true; }
>>
>> +  Stmt::child_range getStmtChildren(Stmt *S) { return S->children(); }
>> +
>> +  SmallVector getStmtChildren(CXXOperatorCallExpr *CE) {
>> +SmallVector Children(CE->children());
>> +bool Swap;
>> +// Switch the operator and the first operand for all infix and postfix
>> +// operations.
>> +switch (CE->getOperator()) {
>> +case OO_Arrow:
>> +case OO_Call:
>> +case OO_Subscript:
>> +  Swap = true;
>> +  break;
>> +case OO_PlusPlus:
>> +case OO_MinusMinus:
>> +  // These are postfix unless there is exactly one argument.
>> +  Swap = Children.size() != 2;
>> +  break;
>> +default:
>> +  Swap = CE->isInfixBinaryOp();
>> +  break;
>> +}
>> +if (Swap && Children.size() > 1)
>> +  std::swap(Children[0], Children[1]);
>> +return Children;
>> +  }
>> +
>>  private:
>>bool TraverseAdditionalLexicallyNestedDeclarations() {
>>  // FIXME: Ideally the gathered declarations and the declarations in
>> the
>>
>> Modified: cfe/trunk/include/clang/AST/RecursiveASTVisitor.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
>> clang/AST/RecursiveASTVisitor.h?rev=312633=312632=312633=diff
>> 

[PATCH] D37662: [AST] Make RecursiveASTVisitor visit TemplateDecls in source order

[Johannes Altmanninger via Phabricator via cfe-commits]

johannes created this revision.

This causes template arguments to be traversed before the templated
declaration, which is useful for clients that expect the nodes in
the same order as they are in the source code. Additionally, there
seems to be no good reason not to do so.

This was moved here from LexicallyOrderedRecursiveASTVisitor. The tests
still reside in LexicallyOrderedRecursiveASTVisitorTest.cpp under
VisitTemplateDecls.


https://reviews.llvm.org/D37662

Files:
  include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h
  include/clang/AST/RecursiveASTVisitor.h


Index: include/clang/AST/RecursiveASTVisitor.h
===
--- include/clang/AST/RecursiveASTVisitor.h
+++ include/clang/AST/RecursiveASTVisitor.h
@@ -537,7 +537,6 @@
 
   bool dataTraverseNode(Stmt *S, DataRecursionQueue *Queue);
   bool PostVisitStmt(Stmt *S);
-  bool shouldTraverseTemplateArgumentsBeforeDecl() const { return false; }
 };
 
 template 
@@ -1691,13 +1690,8 @@
 // template declarations.
 #define DEF_TRAVERSE_TMPL_DECL(TMPLDECLKIND)   
\
   DEF_TRAVERSE_DECL(TMPLDECLKIND##TemplateDecl, {  
\
-if (getDerived().shouldTraverseTemplateArgumentsBeforeDecl()) {
\
-  TRY_TO(TraverseTemplateParameterListHelper(D->getTemplateParameters())); 
\
-  TRY_TO(TraverseDecl(D->getTemplatedDecl())); 
\
-} else {   
\
-  TRY_TO(TraverseDecl(D->getTemplatedDecl())); 
\
-  TRY_TO(TraverseTemplateParameterListHelper(D->getTemplateParameters())); 
\
-}  
\
+TRY_TO(TraverseTemplateParameterListHelper(D->getTemplateParameters()));   
\
+TRY_TO(TraverseDecl(D->getTemplatedDecl()));   
\

\
 /* By default, we do not traverse the instantiations of
\
class templates since they do not appear in the user code. The  
\
Index: include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h
===
--- include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h
+++ include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h
@@ -111,8 +111,6 @@
 return true;
   }
 
-  bool shouldTraverseTemplateArgumentsBeforeDecl() const { return true; }
-
   Stmt::child_range getStmtChildren(Stmt *S) { return S->children(); }
 
   SmallVector getStmtChildren(CXXOperatorCallExpr *CE) {


Index: include/clang/AST/RecursiveASTVisitor.h
===
--- include/clang/AST/RecursiveASTVisitor.h
+++ include/clang/AST/RecursiveASTVisitor.h
@@ -537,7 +537,6 @@
 
   bool dataTraverseNode(Stmt *S, DataRecursionQueue *Queue);
   bool PostVisitStmt(Stmt *S);
-  bool shouldTraverseTemplateArgumentsBeforeDecl() const { return false; }
 };
 
 template 
@@ -1691,13 +1690,8 @@
 // template declarations.
 #define DEF_TRAVERSE_TMPL_DECL(TMPLDECLKIND)   \
   DEF_TRAVERSE_DECL(TMPLDECLKIND##TemplateDecl, {  \
-if (getDerived().shouldTraverseTemplateArgumentsBeforeDecl()) {\
-  TRY_TO(TraverseTemplateParameterListHelper(D->getTemplateParameters())); \
-  TRY_TO(TraverseDecl(D->getTemplatedDecl())); \
-} else {   \
-  TRY_TO(TraverseDecl(D->getTemplatedDecl())); \
-  TRY_TO(TraverseTemplateParameterListHelper(D->getTemplateParameters())); \
-}  \
+TRY_TO(TraverseTemplateParameterListHelper(D->getTemplateParameters()));   \
+TRY_TO(TraverseDecl(D->getTemplatedDecl()));   \
\
 /* By default, we do not traverse the instantiations of\
class templates since they do not appear in the user code. The  \
Index: include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h
===
--- include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h
+++ include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h
@@ -111,8 +111,6 @@
 return true;
   }
 
-  bool shouldTraverseTemplateArgumentsBeforeDecl() const { return true; }
-
   Stmt::child_range getStmtChildren(Stmt *S) { return S->children(); }
 
   SmallVector getStmtChildren(CXXOperatorCallExpr *CE) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37663: [AST] Make RecursiveASTVisitor visit CXXOperatorCallExpr in source order

[Johannes Altmanninger via Phabricator via cfe-commits]

johannes created this revision.

This adds a special case for traversing CXXOperatorCallExpr. Its
children are traversed in the order in which they appear in the source
code, so infix and postfix operators are visited after their first
argument.

This behavior was previously only in
LexicallyOrderedRecursiveASTVisitor, moving it here could avoid bugs in
the future. It is still tested in
LexicallyOrderedRecursiveASTVisitorTest.cpp in VisitTemplateDecl.
Clients that somehow rely on the original traversal order will have to
be updated though.


https://reviews.llvm.org/D37663

Files:
  include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h
  include/clang/AST/RecursiveASTVisitor.h


Index: include/clang/AST/RecursiveASTVisitor.h
===
--- include/clang/AST/RecursiveASTVisitor.h
+++ include/clang/AST/RecursiveASTVisitor.h
@@ -315,15 +315,40 @@
 
 //  Methods on Stmts 
 
-  Stmt::child_range getStmtChildren(Stmt *S) { return S->children(); }
-
 private:
   template
   struct has_same_member_pointer_type : std::false_type {};
   template
   struct has_same_member_pointer_type
   : std::true_type {};
 
+  Stmt::child_range getStmtChildren(Stmt *S) { return S->children(); }
+
+  SmallVector getStmtChildren(CXXOperatorCallExpr *CE) {
+SmallVector Children(CE->children());
+bool Swap;
+// Swap the operator and the first operand for all infix and postfix
+// operations.
+switch (CE->getOperator()) {
+case OO_Arrow:
+case OO_Call:
+case OO_Subscript:
+  Swap = true;
+  break;
+case OO_PlusPlus:
+case OO_MinusMinus:
+  // These are postfix unless there is exactly one argument.
+  Swap = Children.size() != 2;
+  break;
+default:
+  Swap = CE->isInfixBinaryOp();
+  break;
+}
+if (Swap && Children.size() > 1)
+  std::swap(Children[0], Children[1]);
+return Children;
+  }
+
   // Traverse the given statement. If the most-derived traverse function takes 
a
   // data recursion queue, pass it on; otherwise, discard it. Note that the
   // first branch of this conditional must compile whether or not the derived
Index: include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h
===
--- include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h
+++ include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h
@@ -111,33 +111,6 @@
 return true;
   }
 
-  Stmt::child_range getStmtChildren(Stmt *S) { return S->children(); }
-
-  SmallVector getStmtChildren(CXXOperatorCallExpr *CE) {
-SmallVector Children(CE->children());
-bool Swap;
-// Switch the operator and the first operand for all infix and postfix
-// operations.
-switch (CE->getOperator()) {
-case OO_Arrow:
-case OO_Call:
-case OO_Subscript:
-  Swap = true;
-  break;
-case OO_PlusPlus:
-case OO_MinusMinus:
-  // These are postfix unless there is exactly one argument.
-  Swap = Children.size() != 2;
-  break;
-default:
-  Swap = CE->isInfixBinaryOp();
-  break;
-}
-if (Swap && Children.size() > 1)
-  std::swap(Children[0], Children[1]);
-return Children;
-  }
-
 private:
   bool TraverseAdditionalLexicallyNestedDeclarations() {
 // FIXME: Ideally the gathered declarations and the declarations in the


Index: include/clang/AST/RecursiveASTVisitor.h
===
--- include/clang/AST/RecursiveASTVisitor.h
+++ include/clang/AST/RecursiveASTVisitor.h
@@ -315,15 +315,40 @@
 
 //  Methods on Stmts 
 
-  Stmt::child_range getStmtChildren(Stmt *S) { return S->children(); }
-
 private:
   template
   struct has_same_member_pointer_type : std::false_type {};
   template
   struct has_same_member_pointer_type
   : std::true_type {};
 
+  Stmt::child_range getStmtChildren(Stmt *S) { return S->children(); }
+
+  SmallVector getStmtChildren(CXXOperatorCallExpr *CE) {
+SmallVector Children(CE->children());
+bool Swap;
+// Swap the operator and the first operand for all infix and postfix
+// operations.
+switch (CE->getOperator()) {
+case OO_Arrow:
+case OO_Call:
+case OO_Subscript:
+  Swap = true;
+  break;
+case OO_PlusPlus:
+case OO_MinusMinus:
+  // These are postfix unless there is exactly one argument.
+  Swap = Children.size() != 2;
+  break;
+default:
+  Swap = CE->isInfixBinaryOp();
+  break;
+}
+if (Swap && Children.size() > 1)
+  std::swap(Children[0], Children[1]);
+return Children;
+  }
+
   // Traverse the given statement. If the most-derived traverse function takes a
   // data recursion queue, pass it on; otherwise, discard it. Note that the
   // first branch of this conditional must compile whether or not the derived
Index: include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h

[PATCH] D37134: [libc++] Rerun ranlib manually after merging the static libraries

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

Ping @EricWF


https://reviews.llvm.org/D37134



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

[PATCH] D37133: [libc++] Handle object files named *.obj in merge_archives.py

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

Ping @EricWF, who added this script


https://reviews.llvm.org/D37133



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

[PATCH] D34367: CodeGen: Fix address space of indirect function argument

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/CodeGen/CGCall.h:211
+  push_back(CallArg(rvalue, type, needscopy, AS));
 }
 

Ah, I think I understand what's going on here now.  "indirect byval" arguments 
include an implicit copy to the stack, and the code is trying to avoid emitting 
an extra copy to a temporary in the frontend.  It does this by recognizing 
loads from aggregate l-values and just adding the l-value to the call-argument 
list as a "needs copy" argument.  Call-argument preparation then recognizes 
that the argument is being passed indirect byval and, under certain conditions, 
just uses the l-value address as the IR argument.

Instead of creeping more and more information from the LValue into the CallArg, 
I think there are two reasonable alternatives:

- Suppress this optimization in EmitCallArg when the LValue isn't in the alloca 
address space.  EmitCallArg already suppresses it when the argument is 
under-aligned.

- Find a way to actually store an LValue in a CallArg.  This has the advantage 
that we can assert if someone tries to access it as an RValue, which is good 
because any attempt to use a needs-copy argument as if it were a normal 
argument is a lurking bug.


https://reviews.llvm.org/D34367



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

[PATCH] improve help-layout of clang-format

[Bas van den Berg via cfe-commits]

clang-format: improve layout of help message

Signed-off-by: Bas van den Berg 
---
 tools/clang-format/ClangFormat.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/clang-format/ClangFormat.cpp
b/tools/clang-format/ClangFormat.cpp
index 14bff19..6cfce07 100644
--- a/tools/clang-format/ClangFormat.cpp
+++ b/tools/clang-format/ClangFormat.cpp
@@ -98,8 +98,8 @@ static cl::opt

 static cl::opt SortIncludes(
 "sort-includes",
-cl::desc("If set, overrides the include sorting behavior determined by
the "
- "SortIncludes style flag"),
+cl::desc("If set, overrides the include sorting behavior\n
+  determined by the SortIncludes style flag"),
 cl::cat(ClangFormatCategory));

 static cl::list FileNames(cl::Positional, cl::desc("[
...]"),
-- 
2.7.4
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits