Re: [PATCH] D11328: [clang-tidy] new "throw-by-value-catch-by-reference" check for clang-tidy

[Tobias Langner via cfe-commits]

randomcppprogrammer updated this revision to Diff 36700.
randomcppprogrammer marked 17 inline comments as done.
randomcppprogrammer added a comment.

reworked code to include the changes suggested by Aaron Ballman

main changes

- will not diagnose on throwing catch variables by value/pointer
- will not diagnose on throwing function parameters (if you are using special 
handler functions for throwing)


http://reviews.llvm.org/D11328

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
  clang-tidy/misc/ThrowByValueCatchByReferenceCheck.h
  test/clang-tidy/misc-throw-by-value-catch-by-reference.cpp

Index: test/clang-tidy/misc-throw-by-value-catch-by-reference.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-throw-by-value-catch-by-reference.cpp
@@ -0,0 +1,149 @@
+// RUN: %python %S/check_clang_tidy.py %s misc-throw-by-value-catch-by-reference %t
+
+
+class logic_error {
+public:
+  logic_error(const char *message) {}
+};
+
+typedef logic_error *logic_ptr;
+typedef logic_ptr logic_double_typedef;
+
+int lastException;
+
+template  struct remove_reference  {typedef T type;};
+template  struct remove_reference  {typedef T type;};
+template  struct remove_reference {typedef T type;};
+
+template 
+typename remove_reference::type&& move(T&& arg) {
+  return static_cast::type&&>(arg);
+}
+
+logic_error CreateException() { return logic_error("created"); }
+
+void testThrowFunc() {
+  throw new logic_error("by pointer");
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: throw expression throws a pointer; it should throw a non pointer value instead [misc-throw-by-value-catch-by-reference]
+  logic_ptr tmp = new logic_error("by pointer");
+  throw tmp;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: throw expression should throw anonymous temporary values instead [misc-throw-by-value-catch-by-reference]
+  // CHECK-MESSAGES: :[[@LINE-2]]:9: warning: throw expression throws a pointer; it should throw a non pointer value instead [misc-throw-by-value-catch-by-reference]
+  throw logic_error("by value");
+  auto *literal = "test";
+  throw logic_error(literal);
+  throw "test string literal";
+  throw L"throw wide string literal";
+  const char *characters = 0;
+  throw characters;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: throw expression should throw anonymous temporary values instead [misc-throw-by-value-catch-by-reference]
+  // CHECK-MESSAGES: :[[@LINE-2]]:9: warning: throw expression throws a pointer; it should throw a non pointer value instead [misc-throw-by-value-catch-by-reference]
+   logic_error lvalue("lvalue");
+  throw lvalue;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: throw expression should throw anonymous temporary values instead [misc-throw-by-value-catch-by-reference]
+  
+  // can be enabled once std::move can be included
+  throw move(lvalue);  
+  int  = lastException;
+  throw ex;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: throw expression should throw anonymous temporary values instead [misc-throw-by-value-catch-by-reference]
+  throw CreateException();
+}
+
+void throwReferenceFunc(logic_error ) {
+  throw ref;
+}
+
+void catchByPointer() {
+  try {
+testThrowFunc();
+  } catch (logic_error *e) {
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: catch handler catches a pointer value; should throw a non-pointer value and catch by reference instead [misc-throw-by-value-catch-by-reference]
+  }
+}
+
+void catchByValue() {
+  try {
+testThrowFunc();
+  } catch (logic_error e) {
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: catch handler catches a pointer value; should throw a non-pointer value and catch by reference instead [misc-throw-by-value-catch-by-reference]
+  }
+}
+
+void catchByReference() {
+  try {
+testThrowFunc();
+  } catch (logic_error ) {
+  }
+}
+
+void catchByConstReference() {
+  try {
+testThrowFunc();
+  } catch (const logic_error ) {
+  }
+}
+
+void catchTypedef() {
+  try {
+testThrowFunc();
+  } catch (logic_ptr) {
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: catch handler catches a pointer value; should throw a non-pointer value and catch by reference instead [misc-throw-by-value-catch-by-reference]
+  }
+}
+
+void catchAll() {
+  try {
+testThrowFunc();
+  } catch (...) {
+  }
+}
+
+void catchLiteral() {
+  try {
+testThrowFunc();
+  } catch (const char *) {
+  } catch (const wchar_t *) {
+// disabled for now until it is clear
+// how to enable them in the test
+//} catch (const char16_t*) {
+//} catch (const char32_t*) {
+  }
+}
+
+// catching fundamentals should not warn
+void catchFundamental() {
+  try {
+testThrowFunc();
+  } catch (int) {
+  } catch (double) {
+  } catch (unsigned long) {
+  }
+}
+
+struct TrivialType {
+  double x;
+  double y;
+};
+
+void catchTrivial() {
+  try {
+testThrowFunc();
+  } catch (TrivialType) {
+  }
+}
+

Re: [PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

[Aaron Ballman via cfe-commits]

On Tue, Oct 6, 2015 at 10:19 AM, Manuel Klimek  wrote:
> On Tue, Oct 6, 2015 at 4:18 PM Aaron Ballman 
> wrote:
>>
>> On Tue, Oct 6, 2015 at 10:15 AM, Manuel Klimek  wrote:
>> > On Tue, Oct 6, 2015 at 4:12 PM Aaron Ballman 
>> > wrote:
>> >>
>> >> aaron.ballman added a comment.
>> >>
>> >> In http://reviews.llvm.org/D13368#260672, @klimek wrote:
>> >>
>> >> > In http://reviews.llvm.org/D13368#260669, @aaron.ballman wrote:
>> >> >
>> >> > > This wasn't a comment on the rule so much as a comment on the
>> >> > > diagnostic not being very helpful.In this case, you're telling the
>> >> > > user to
>> >> > > not do something, but it is unclear how the user would structure
>> >> > > their code
>> >> > > to silence this diagnostic. Perhaps there is no way to word this to
>> >> > > give the
>> >> > > user a clue, but we should at least try. If I got this diagnostic
>> >> > > as it is
>> >> > > now, I would scratch my head and quickly decide to ignore it.
>> >> >
>> >> >
>> >> > The cpp core guidelines are written in a way that they should be
>> >> > referenceable by links - do we want to add links to the relevant
>> >> > portions of
>> >> > the core guidelines from the clang-tidy checks?
>> >>
>> >>
>> >> I'd be hesitant to do that. It would add a lot of verbiage to
>> >> diagnostics
>> >> that are likely to be chatty, and if the links ever go dead mid-release
>> >> cycle for us, we're stuck looking bad with no way to fix it. CERT's
>> >> guidelines are also linkable in the same fashion (as would be
>> >> hypothetical
>> >> checks for MISRA, JSF, etc), and I would have the same hesitation for
>> >> those
>> >> as well due to the potential dead link issue.
>> >>
>> >> I think that having the links within the user-facing documentation is a
>> >> must-have though (and something we've been pretty good about thus far)
>> >> because those can be updated live from ToT. So perhaps a permanent
>> >> short
>> >> link to our own documentation might be useful (if a bit obtuse since
>> >> our
>> >> docs mostly just point to other docs elsewhere)? I'd still be a bit
>> >> worried
>> >> about expired short links or something, but maybe we already host a
>> >> service
>> >> for this sort of thing?
>> >
>> >
>> > I'll postulate that a) github will not go away anytime soon and b)
>> > github
>> > will have a hard time changing their link structure so linking into
>> > revision
>> > N in branch M doesn't work any more.
>> > Thus, I think if we link into the github release of the core
>> > guildelines,
>> > we'll be fine.
>>
>> Github's structure and stability isn't what I'm worried about. The C++
>> Core Guidelines internal structure is what I am worried about. They
>> currently use anchors to navigate around CppCoreGuidelines.md, and
>> those anchor names may or may not be stable. Even with the best of
>> intentions on stability, links change.
>
>
> Can't we link it to one specific version in time, and update the base
> revision when we did QA on the links?

Yes, we could link it to the git hash, but then we would likely want
to shorten the links for display to the user (perhaps as a service off
llvm.org so we can control the rewrites with more granularity).

I think this is a good idea that should have some broader community
discussion to iron out the details.

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

r249409 - [VFS] Put the incoming name in the file status to make InMemoryFS behave more like a real FS.

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Tue Oct  6 09:45:16 2015
New Revision: 249409

URL: http://llvm.org/viewvc/llvm-project?rev=249409=rev
Log:
[VFS] Put the incoming name in the file status to make InMemoryFS behave more 
like a real FS.

Modified:
cfe/trunk/lib/Basic/VirtualFileSystem.cpp
cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp

Modified: cfe/trunk/lib/Basic/VirtualFileSystem.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/VirtualFileSystem.cpp?rev=249409=249408=249409=diff
==
--- cfe/trunk/lib/Basic/VirtualFileSystem.cpp (original)
+++ cfe/trunk/lib/Basic/VirtualFileSystem.cpp Tue Oct  6 09:45:16 2015
@@ -501,7 +501,7 @@ void InMemoryFileSystem::addFile(const T
   if (I == E) {
 // End of the path, create a new file.
 // FIXME: expose the status details in the interface.
-Status Stat(Path, getNextVirtualUniqueID(),
+Status Stat(P.str(), getNextVirtualUniqueID(),
 llvm::sys::TimeValue(ModificationTime, 0), 0, 0,
 Buffer->getBufferSize(),
 llvm::sys::fs::file_type::regular_file,

Modified: cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp?rev=249409=249408=249409=diff
==
--- cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp (original)
+++ cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp Tue Oct  6 09:45:16 2015
@@ -602,6 +602,19 @@ TEST_F(InMemoryFileSystemTest, Directory
   ASSERT_EQ(vfs::directory_iterator(), I);
 }
 
+TEST_F(InMemoryFileSystemTest, WorkingDirectory) {
+  FS.setCurrentWorkingDirectory("/b");
+  FS.addFile("c", 0, MemoryBuffer::getMemBuffer(""));
+
+  auto Stat = FS.status("/b/c");
+  ASSERT_FALSE(Stat.getError()) << Stat.getError() << "\n" << FS.toString();
+  ASSERT_EQ("c", Stat->getName());
+  ASSERT_EQ("/b", *FS.getCurrentWorkingDirectory());
+
+  Stat = FS.status("c");
+  ASSERT_FALSE(Stat.getError()) << Stat.getError() << "\n" << FS.toString();
+}
+
 // NOTE: in the tests below, we use '//root/' as our root directory, since it 
is
 // a legal *absolute* path on Windows as well as *nix.
 class VFSFromYAMLTest : public ::testing::Test {


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

r249408 - [Tooling] Remove dead code.

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Tue Oct  6 09:45:13 2015
New Revision: 249408

URL: http://llvm.org/viewvc/llvm-project?rev=249408=rev
Log:
[Tooling] Remove dead code.

It took me some time to figure out why this is not working as expected:
std:error_code converts to true if there is an error. This means we never
ever took the generated absolute path, which happens to be the right thing
anyways as it properly works with virtual files. Just remove the whole
thing, relative paths are covered by existing tooling tests.

Modified:
cfe/trunk/lib/Tooling/Core/Replacement.cpp

Modified: cfe/trunk/lib/Tooling/Core/Replacement.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Core/Replacement.cpp?rev=249408=249407=249408=diff
==
--- cfe/trunk/lib/Tooling/Core/Replacement.cpp (original)
+++ cfe/trunk/lib/Tooling/Core/Replacement.cpp Tue Oct  6 09:45:13 2015
@@ -113,15 +113,7 @@ void Replacement::setFromSourceLocation(
   const std::pair DecomposedLocation =
   Sources.getDecomposedLoc(Start);
   const FileEntry *Entry = Sources.getFileEntryForID(DecomposedLocation.first);
-  if (Entry) {
-// Make FilePath absolute so replacements can be applied correctly when
-// relative paths for files are used.
-llvm::SmallString<256> FilePath(Entry->getName());
-std::error_code EC = llvm::sys::fs::make_absolute(FilePath);
-this->FilePath = EC ? FilePath.c_str() : Entry->getName();
-  } else {
-this->FilePath = InvalidLocation;
-  }
+  this->FilePath = Entry ? Entry->getName() : InvalidLocation;
   this->ReplacementRange = Range(DecomposedLocation.second, Length);
   this->ReplacementText = ReplacementText;
 }


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

Re: r249395 - BasicTests: Suppress InMemoryFileSystemTest.WindowsPath on win32 while investigating.

[Rafael Espíndola via cfe-commits]

What was the error?

On 6 October 2015 at 08:16, NAKAMURA Takumi via cfe-commits
 wrote:
> Author: chapuni
> Date: Tue Oct  6 07:16:27 2015
> New Revision: 249395
>
> URL: http://llvm.org/viewvc/llvm-project?rev=249395=rev
> Log:
> BasicTests: Suppress InMemoryFileSystemTest.WindowsPath on win32 while 
> investigating.
>
> Modified:
> cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
>
> Modified: cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp?rev=249395=249394=249395=diff
> ==
> --- cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp (original)
> +++ cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp Tue Oct  6 07:16:27 
> 2015
> @@ -536,7 +536,9 @@ TEST_F(InMemoryFileSystemTest, IsEmpty)
>  TEST_F(InMemoryFileSystemTest, WindowsPath) {
>FS.addFile("c:/windows/system128/foo.cpp", 0, 
> MemoryBuffer::getMemBuffer(""));
>auto Stat = FS.status("c:");
> +#if !defined(_WIN32)
>ASSERT_FALSE(Stat.getError()) << Stat.getError() << FS.toString();
> +#endif
>Stat = FS.status("c:/windows/system128/foo.cpp");
>ASSERT_FALSE(Stat.getError()) << Stat.getError() << FS.toString();
>FS.addFile("d:/windows/foo.cpp", 0, MemoryBuffer::getMemBuffer(""));
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r249410 - [Tooling] Reuse FileManager in ASTUnit.

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Tue Oct  6 09:45:20 2015
New Revision: 249410

URL: http://llvm.org/viewvc/llvm-project?rev=249410=rev
Log:
[Tooling] Reuse FileManager in ASTUnit.

ASTUnit was creating multiple FileManagers and throwing them away. Reuse
the one from Tooling. No functionality change now but necessary for
VFSifying tooling.

Modified:
cfe/trunk/include/clang/Frontend/ASTUnit.h
cfe/trunk/lib/Frontend/ASTUnit.cpp
cfe/trunk/lib/Tooling/Tooling.cpp

Modified: cfe/trunk/include/clang/Frontend/ASTUnit.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/ASTUnit.h?rev=249410=249409=249410=diff
==
--- cfe/trunk/include/clang/Frontend/ASTUnit.h (original)
+++ cfe/trunk/include/clang/Frontend/ASTUnit.h Tue Oct  6 09:45:20 2015
@@ -805,9 +805,9 @@ public:
   static std::unique_ptr LoadFromCompilerInvocation(
   CompilerInvocation *CI,
   std::shared_ptr PCHContainerOps,
-  IntrusiveRefCntPtr Diags, bool OnlyLocalDecls = false,
-  bool CaptureDiagnostics = false, bool PrecompilePreamble = false,
-  TranslationUnitKind TUKind = TU_Complete,
+  IntrusiveRefCntPtr Diags, FileManager *FileMgr,
+  bool OnlyLocalDecls = false, bool CaptureDiagnostics = false,
+  bool PrecompilePreamble = false, TranslationUnitKind TUKind = 
TU_Complete,
   bool CacheCodeCompletionResults = false,
   bool IncludeBriefCommentsInCodeCompletion = false,
   bool UserFilesAreVolatile = false);

Modified: cfe/trunk/lib/Frontend/ASTUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ASTUnit.cpp?rev=249410=249409=249410=diff
==
--- cfe/trunk/lib/Frontend/ASTUnit.cpp (original)
+++ cfe/trunk/lib/Frontend/ASTUnit.cpp Tue Oct  6 09:45:20 2015
@@ -1076,11 +1076,10 @@ bool ASTUnit::Parse(std::shared_ptrgetInvocation().LangOpts;
   FileSystemOpts = Clang->getFileSystemOpts();
-  IntrusiveRefCntPtr VFS =
-  createVFSFromCompilerInvocation(Clang->getInvocation(), 
getDiagnostics());
-  if (!VFS)
-return true;
-  FileMgr = new FileManager(FileSystemOpts, VFS);
+  if (!FileMgr) {
+Clang->createFileManager();
+FileMgr = >getFileManager();
+  }
   SourceMgr = new SourceManager(getDiagnostics(), *FileMgr,
 UserFilesAreVolatile);
   TheSema.reset();
@@ -1892,8 +1891,8 @@ bool ASTUnit::LoadFromCompilerInvocation
 std::unique_ptr ASTUnit::LoadFromCompilerInvocation(
 CompilerInvocation *CI,
 std::shared_ptr PCHContainerOps,
-IntrusiveRefCntPtr Diags, bool OnlyLocalDecls,
-bool CaptureDiagnostics, bool PrecompilePreamble,
+IntrusiveRefCntPtr Diags, FileManager *FileMgr,
+bool OnlyLocalDecls, bool CaptureDiagnostics, bool PrecompilePreamble,
 TranslationUnitKind TUKind, bool CacheCodeCompletionResults,
 bool IncludeBriefCommentsInCodeCompletion, bool UserFilesAreVolatile) {
   // Create the AST unit.
@@ -1907,12 +1906,8 @@ std::unique_ptr ASTUnit::LoadFr
   AST->IncludeBriefCommentsInCodeCompletion
 = IncludeBriefCommentsInCodeCompletion;
   AST->Invocation = CI;
-  AST->FileSystemOpts = CI->getFileSystemOpts();
-  IntrusiveRefCntPtr VFS =
-  createVFSFromCompilerInvocation(*CI, *Diags);
-  if (!VFS)
-return nullptr;
-  AST->FileMgr = new FileManager(AST->FileSystemOpts, VFS);
+  AST->FileSystemOpts = FileMgr->getFileSystemOpts();
+  AST->FileMgr = FileMgr;
   AST->UserFilesAreVolatile = UserFilesAreVolatile;
   
   // Recover resources if we crash before exiting this method.
@@ -2043,6 +2038,7 @@ bool ASTUnit::Reparse(std::shared_ptrgetDiagnosticOpts());
   if (OverrideMainBuffer)

Modified: cfe/trunk/lib/Tooling/Tooling.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Tooling.cpp?rev=249410=249409=249410=diff
==
--- cfe/trunk/lib/Tooling/Tooling.cpp (original)
+++ cfe/trunk/lib/Tooling/Tooling.cpp Tue Oct  6 09:45:20 2015
@@ -418,12 +418,12 @@ public:
   bool runInvocation(CompilerInvocation *Invocation, FileManager *Files,
  std::shared_ptr PCHContainerOps,
  DiagnosticConsumer *DiagConsumer) override {
-// FIXME: This should use the provided FileManager.
 std::unique_ptr AST = ASTUnit::LoadFromCompilerInvocation(
 Invocation, PCHContainerOps,
 CompilerInstance::createDiagnostics(>getDiagnosticOpts(),
 DiagConsumer,
-/*ShouldOwnClient=*/false));
+/*ShouldOwnClient=*/false),
+Files);
 if (!AST)
   return false;
 


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

Re: [PATCH] D13444: [clang-tidy] Clocky module and multiple check from my repository

[Aaron Ballman via cfe-commits]

On Tue, Oct 6, 2015 at 8:56 AM, Piotr Zegar  wrote:
> ClockMan abandoned this revision.
> ClockMan added a comment.
>
> As a 'corporation' in which I work has doubts that checks developed by my 
> after work, but tested on copyright protected code should be released to 
> public under my 'name'. I will drop "push request", until everything will 
> clarify.

I look forward to seeing a new patch at some point. I agree with
Eugene that any future patch should be split out into distinct patches
that focus on just one check at a time.

> As for author name in module: For me is a: must-have, as I plan to extend 
> these checks and develop more.
> As a author name in check name: I'm thinking about some "tags", so check 
> could be registered under multiple names: clocky-object-copy-in-range-for and 
> performance-object-copy-in-range-for.

I would be opposed to using an author name in anything that is
user-facing. We group the checkers either by functionality
("modernize-*" and "readability-*"), by organizational name
("google-*" and "llvm-*"), or by publication name ("cert-* and
"cppcoreguidelines-*"). We actively discourage use of author names
even in our code comments.

~Aaron

>
> Best regards,
> Clocky
>
>
> Repository:
>   rL LLVM
>
> http://reviews.llvm.org/D13444
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12839: Extend MoveConstructorInitCheck to also flag constructor arguments passed by value and can be moved assigned to fields.

[Aaron Ballman via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM!

Do you have commit access, or would you like me to commit on your behalf?


http://reviews.llvm.org/D12839



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

Re: [PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

[Manuel Klimek via cfe-commits]

On Tue, Oct 6, 2015 at 4:12 PM Aaron Ballman 
wrote:

> aaron.ballman added a comment.
>
> In http://reviews.llvm.org/D13368#260672, @klimek wrote:
>
> > In http://reviews.llvm.org/D13368#260669, @aaron.ballman wrote:
> >
> > > This wasn't a comment on the rule so much as a comment on the
> diagnostic not being very helpful.In this case, you're telling the user to
> not do something, but it is unclear how the user would structure their code
> to silence this diagnostic. Perhaps there is no way to word this to give
> the user a clue, but we should at least try. If I got this diagnostic as it
> is now, I would scratch my head and quickly decide to ignore it.
> >
> >
> > The cpp core guidelines are written in a way that they should be
> referenceable by links - do we want to add links to the relevant portions
> of the core guidelines from the clang-tidy checks?
>
>
> I'd be hesitant to do that. It would add a lot of verbiage to diagnostics
> that are likely to be chatty, and if the links ever go dead mid-release
> cycle for us, we're stuck looking bad with no way to fix it. CERT's
> guidelines are also linkable in the same fashion (as would be hypothetical
> checks for MISRA, JSF, etc), and I would have the same hesitation for those
> as well due to the potential dead link issue.
>
> I think that having the links within the user-facing documentation is a
> must-have though (and something we've been pretty good about thus far)
> because those can be updated live from ToT. So perhaps a permanent short
> link to our own documentation might be useful (if a bit obtuse since our
> docs mostly just point to other docs elsewhere)? I'd still be a bit worried
> about expired short links or something, but maybe we already host a service
> for this sort of thing?


I'll postulate that a) github will not go away anytime soon and b) github
will have a hard time changing their link structure so linking into
revision N in branch M doesn't work any more.
Thus, I think if we link into the github release of the core guildelines,
we'll be fine.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

[Aaron Ballman via cfe-commits]

On Tue, Oct 6, 2015 at 10:15 AM, Manuel Klimek  wrote:
> On Tue, Oct 6, 2015 at 4:12 PM Aaron Ballman 
> wrote:
>>
>> aaron.ballman added a comment.
>>
>> In http://reviews.llvm.org/D13368#260672, @klimek wrote:
>>
>> > In http://reviews.llvm.org/D13368#260669, @aaron.ballman wrote:
>> >
>> > > This wasn't a comment on the rule so much as a comment on the
>> > > diagnostic not being very helpful.In this case, you're telling the user 
>> > > to
>> > > not do something, but it is unclear how the user would structure their 
>> > > code
>> > > to silence this diagnostic. Perhaps there is no way to word this to give 
>> > > the
>> > > user a clue, but we should at least try. If I got this diagnostic as it 
>> > > is
>> > > now, I would scratch my head and quickly decide to ignore it.
>> >
>> >
>> > The cpp core guidelines are written in a way that they should be
>> > referenceable by links - do we want to add links to the relevant portions 
>> > of
>> > the core guidelines from the clang-tidy checks?
>>
>>
>> I'd be hesitant to do that. It would add a lot of verbiage to diagnostics
>> that are likely to be chatty, and if the links ever go dead mid-release
>> cycle for us, we're stuck looking bad with no way to fix it. CERT's
>> guidelines are also linkable in the same fashion (as would be hypothetical
>> checks for MISRA, JSF, etc), and I would have the same hesitation for those
>> as well due to the potential dead link issue.
>>
>> I think that having the links within the user-facing documentation is a
>> must-have though (and something we've been pretty good about thus far)
>> because those can be updated live from ToT. So perhaps a permanent short
>> link to our own documentation might be useful (if a bit obtuse since our
>> docs mostly just point to other docs elsewhere)? I'd still be a bit worried
>> about expired short links or something, but maybe we already host a service
>> for this sort of thing?
>
>
> I'll postulate that a) github will not go away anytime soon and b) github
> will have a hard time changing their link structure so linking into revision
> N in branch M doesn't work any more.
> Thus, I think if we link into the github release of the core guildelines,
> we'll be fine.

Github's structure and stability isn't what I'm worried about. The C++
Core Guidelines internal structure is what I am worried about. They
currently use anchors to navigate around CppCoreGuidelines.md, and
those anchor names may or may not be stable. Even with the best of
intentions on stability, links change.

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

[PATCH] D13474: [VFS] Port tooling to use the in-memory file system.

[Benjamin Kramer via cfe-commits]

bkramer created this revision.
bkramer added a reviewer: klimek.
bkramer added a subscriber: cfe-commits.
Herald added a subscriber: klimek.

This means file remappings can now be managed by ClangTool (or a
ToolInvocation user) instead of by ToolInvocation itself. The
ToolInvocation remapping is still in place so users can migrate.

http://reviews.llvm.org/D13474

Files:
  include/clang/Tooling/Tooling.h
  lib/Tooling/Tooling.cpp
  unittests/Tooling/ToolingTest.cpp

Index: unittests/Tooling/ToolingTest.cpp
===
--- unittests/Tooling/ToolingTest.cpp
+++ unittests/Tooling/ToolingTest.cpp
@@ -149,39 +149,54 @@
 }
 
 TEST(ToolInvocation, TestMapVirtualFile) {
-  IntrusiveRefCntPtr Files(
-  new clang::FileManager(clang::FileSystemOptions()));
+  llvm::IntrusiveRefCntPtr OverlayFileSystem(
+  new vfs::OverlayFileSystem(vfs::getRealFileSystem()));
+  llvm::IntrusiveRefCntPtr InMemoryFileSystem(
+  new vfs::InMemoryFileSystem);
+  OverlayFileSystem->pushOverlay(InMemoryFileSystem);
+  llvm::IntrusiveRefCntPtr Files(
+  new FileManager(FileSystemOptions(), OverlayFileSystem));
   std::vector Args;
   Args.push_back("tool-executable");
   Args.push_back("-Idef");
   Args.push_back("-fsyntax-only");
   Args.push_back("test.cpp");
   clang::tooling::ToolInvocation Invocation(Args, new SyntaxOnlyAction,
 Files.get());
-  Invocation.mapVirtualFile("test.cpp", "#include \n");
-  Invocation.mapVirtualFile("def/abc", "\n");
+  InMemoryFileSystem->addFile(
+  "test.cpp", 0, llvm::MemoryBuffer::getMemBuffer("#include \n"));
+  InMemoryFileSystem->addFile("def/abc", 0,
+  llvm::MemoryBuffer::getMemBuffer("\n"));
   EXPECT_TRUE(Invocation.run());
 }
 
 TEST(ToolInvocation, TestVirtualModulesCompilation) {
   // FIXME: Currently, this only tests that we don't exit with an error if a
   // mapped module.map is found on the include path. In the future, expand this
   // test to run a full modules enabled compilation, so we make sure we can
   // rerun modules compilations with a virtual file system.
-  IntrusiveRefCntPtr Files(
-  new clang::FileManager(clang::FileSystemOptions()));
+  llvm::IntrusiveRefCntPtr OverlayFileSystem(
+  new vfs::OverlayFileSystem(vfs::getRealFileSystem()));
+  llvm::IntrusiveRefCntPtr InMemoryFileSystem(
+  new vfs::InMemoryFileSystem);
+  OverlayFileSystem->pushOverlay(InMemoryFileSystem);
+  llvm::IntrusiveRefCntPtr Files(
+  new FileManager(FileSystemOptions(), OverlayFileSystem));
   std::vector Args;
   Args.push_back("tool-executable");
   Args.push_back("-Idef");
   Args.push_back("-fsyntax-only");
   Args.push_back("test.cpp");
   clang::tooling::ToolInvocation Invocation(Args, new SyntaxOnlyAction,
 Files.get());
-  Invocation.mapVirtualFile("test.cpp", "#include \n");
-  Invocation.mapVirtualFile("def/abc", "\n");
+  InMemoryFileSystem->addFile(
+  "test.cpp", 0, llvm::MemoryBuffer::getMemBuffer("#include \n"));
+  InMemoryFileSystem->addFile("def/abc", 0,
+  llvm::MemoryBuffer::getMemBuffer("\n"));
   // Add a module.map file in the include directory of our header, so we trigger
   // the module.map header search logic.
-  Invocation.mapVirtualFile("def/module.map", "\n");
+  InMemoryFileSystem->addFile("def/module.map", 0,
+  llvm::MemoryBuffer::getMemBuffer("\n"));
   EXPECT_TRUE(Invocation.run());
 }
 
Index: lib/Tooling/Tooling.cpp
===
--- lib/Tooling/Tooling.cpp
+++ lib/Tooling/Tooling.cpp
@@ -32,13 +32,6 @@
 #include "llvm/Support/Host.h"
 #include "llvm/Support/raw_ostream.h"
 
-// For chdir, see the comment in ClangTool::run for more information.
-#ifdef LLVM_ON_WIN32
-#  include 
-#else
-#  include 
-#endif
-
 #define DEBUG_TYPE "clang-tooling"
 
 namespace clang {
@@ -131,18 +124,25 @@
 
   SmallString<16> FileNameStorage;
   StringRef FileNameRef = FileName.toNullTerminatedStringRef(FileNameStorage);
+  llvm::IntrusiveRefCntPtr OverlayFileSystem(
+  new vfs::OverlayFileSystem(vfs::getRealFileSystem()));
+  llvm::IntrusiveRefCntPtr InMemoryFileSystem(
+  new vfs::InMemoryFileSystem);
+  OverlayFileSystem->pushOverlay(InMemoryFileSystem);
   llvm::IntrusiveRefCntPtr Files(
-  new FileManager(FileSystemOptions()));
+  new FileManager(FileSystemOptions(), OverlayFileSystem));
   ToolInvocation Invocation(getSyntaxOnlyToolArgs(Args, FileNameRef),
 ToolAction, Files.get(), PCHContainerOps);
 
   SmallString<1024> CodeStorage;
-  Invocation.mapVirtualFile(FileNameRef,
-Code.toNullTerminatedStringRef(CodeStorage));
+  InMemoryFileSystem->addFile(FileNameRef, 0,
+  llvm::MemoryBuffer::getMemBuffer(
+  

Re: [PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

[Manuel Klimek via cfe-commits]

On Tue, Oct 6, 2015 at 4:18 PM Aaron Ballman 
wrote:

> On Tue, Oct 6, 2015 at 10:15 AM, Manuel Klimek  wrote:
> > On Tue, Oct 6, 2015 at 4:12 PM Aaron Ballman 
> > wrote:
> >>
> >> aaron.ballman added a comment.
> >>
> >> In http://reviews.llvm.org/D13368#260672, @klimek wrote:
> >>
> >> > In http://reviews.llvm.org/D13368#260669, @aaron.ballman wrote:
> >> >
> >> > > This wasn't a comment on the rule so much as a comment on the
> >> > > diagnostic not being very helpful.In this case, you're telling the
> user to
> >> > > not do something, but it is unclear how the user would structure
> their code
> >> > > to silence this diagnostic. Perhaps there is no way to word this to
> give the
> >> > > user a clue, but we should at least try. If I got this diagnostic
> as it is
> >> > > now, I would scratch my head and quickly decide to ignore it.
> >> >
> >> >
> >> > The cpp core guidelines are written in a way that they should be
> >> > referenceable by links - do we want to add links to the relevant
> portions of
> >> > the core guidelines from the clang-tidy checks?
> >>
> >>
> >> I'd be hesitant to do that. It would add a lot of verbiage to
> diagnostics
> >> that are likely to be chatty, and if the links ever go dead mid-release
> >> cycle for us, we're stuck looking bad with no way to fix it. CERT's
> >> guidelines are also linkable in the same fashion (as would be
> hypothetical
> >> checks for MISRA, JSF, etc), and I would have the same hesitation for
> those
> >> as well due to the potential dead link issue.
> >>
> >> I think that having the links within the user-facing documentation is a
> >> must-have though (and something we've been pretty good about thus far)
> >> because those can be updated live from ToT. So perhaps a permanent short
> >> link to our own documentation might be useful (if a bit obtuse since our
> >> docs mostly just point to other docs elsewhere)? I'd still be a bit
> worried
> >> about expired short links or something, but maybe we already host a
> service
> >> for this sort of thing?
> >
> >
> > I'll postulate that a) github will not go away anytime soon and b) github
> > will have a hard time changing their link structure so linking into
> revision
> > N in branch M doesn't work any more.
> > Thus, I think if we link into the github release of the core guildelines,
> > we'll be fine.
>
> Github's structure and stability isn't what I'm worried about. The C++
> Core Guidelines internal structure is what I am worried about. They
> currently use anchors to navigate around CppCoreGuidelines.md, and
> those anchor names may or may not be stable. Even with the best of
> intentions on stability, links change.
>

Can't we link it to one specific version in time, and update the base
revision when we did QA on the links?


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

Re: [PATCH] D13398: [clang-tidy] add check cppcoreguidelines-pro-type-const-cast

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

It looks like potentially we're going to have tens of checks in this module. I 
wonder whether we should start organizing the checks somehow right away. For 
example, create a directory (and a namespace) for each profile. We'd need to 
adapt the add_new_check.py script to support this then.

Just a thought. The patch LG.


http://reviews.llvm.org/D13398



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

r249517 - clang-format: Fix false ObjC block detection.

[Daniel Jasper via cfe-commits]

Author: djasper
Date: Tue Oct  6 22:43:10 2015
New Revision: 249517

URL: http://llvm.org/viewvc/llvm-project?rev=249517=rev
Log:
clang-format: Fix false ObjC block detection.

Before:
  inline A operator^(const A , const A ) {} int i;

After:
  inline A operator^(const A , const A ) {}
  int i;

Modified:
cfe/trunk/lib/Format/UnwrappedLineParser.cpp
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=249517=249516=249517=diff
==
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original)
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Tue Oct  6 22:43:10 2015
@@ -855,6 +855,11 @@ void UnwrappedLineParser::parseStructura
 case tok::l_paren:
   parseParens();
   break;
+case tok::kw_operator:
+  nextToken();
+  if (FormatTok->isBinaryOperator())
+nextToken();
+  break;
 case tok::caret:
   nextToken();
   if (FormatTok->Tok.isAnyIdentifier() ||

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=249517=249516=249517=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Tue Oct  6 22:43:10 2015
@@ -5327,6 +5327,8 @@ TEST_F(FormatTest, UnderstandsOverloaded
   verifyGoogleFormat("operator ::A();");
 
   verifyFormat("using A::operator+;");
+  verifyFormat("inline A operator^(const A , const A ) {}\n"
+   "int i;");
 }
 
 TEST_F(FormatTest, UnderstandsFunctionRefQualification) {


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

r249519 - Make clang-format actually respect custom brace wrapping flags.

[Daniel Jasper via cfe-commits]

Author: djasper
Date: Tue Oct  6 23:06:10 2015
New Revision: 249519

URL: http://llvm.org/viewvc/llvm-project?rev=249519=rev
Log:
Make clang-format actually respect custom brace wrapping flags.

This fixes llvm.org/PR25073.

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

Modified: cfe/trunk/lib/Format/Format.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=249519=249518=249519=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Tue Oct  6 23:06:10 2015
@@ -372,6 +372,8 @@ std::string ParseErrorCategory::message(
 }
 
 static FormatStyle expandPresets(const FormatStyle ) {
+  if (Style.BreakBeforeBraces == FormatStyle::BS_Custom)
+return Style;
   FormatStyle Expanded = Style;
   Expanded.BraceWrapping = {false, false, false, false, false, false,
 false, false, false, false, false};
@@ -442,6 +444,8 @@ FormatStyle getLLVMStyle() {
   LLVMStyle.BreakBeforeBinaryOperators = FormatStyle::BOS_None;
   LLVMStyle.BreakBeforeTernaryOperators = true;
   LLVMStyle.BreakBeforeBraces = FormatStyle::BS_Attach;
+  LLVMStyle.BraceWrapping = {false, false, false, false, false, false,
+ false, false, false, false, false};
   LLVMStyle.BreakConstructorInitializersBeforeComma = false;
   LLVMStyle.ColumnLimit = 80;
   LLVMStyle.CommentPragmas = "^ IWYU pragma:";

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=249519=249518=249519=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Tue Oct  6 23:06:10 2015
@@ -2332,8 +2332,8 @@ TEST_F(FormatTest, IncompleteTryCatchBlo
 
 TEST_F(FormatTest, FormatTryCatchBraceStyles) {
   FormatStyle Style = getLLVMStyle();
-  for (auto BraceStyle :
-   {FormatStyle::BS_Attach, FormatStyle::BS_Mozilla, 
FormatStyle::BS_WebKit}) {
+  for (auto BraceStyle : {FormatStyle::BS_Attach, FormatStyle::BS_Mozilla,
+  FormatStyle::BS_WebKit}) {
 Style.BreakBeforeBraces = BraceStyle;
 verifyFormat("try {\n"
  "  // something\n"
@@ -2384,6 +2384,15 @@ TEST_F(FormatTest, FormatTryCatchBraceSt
"// something\n"
"  }",
Style);
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.BeforeCatch = true;
+  verifyFormat("try {\n"
+   "  // something\n"
+   "}\n"
+   "catch (...) {\n"
+   "  // something\n"
+   "}",
+   Style);
 }
 
 TEST_F(FormatTest, FormatObjCTryCatch) {


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

r249437 - Make clang_Cursor_getMangling don't mangle if the declaration isn't mangled

[Ehsan Akhgari via cfe-commits]

Author: ehsan
Date: Tue Oct  6 13:24:33 2015
New Revision: 249437

URL: http://llvm.org/viewvc/llvm-project?rev=249437=rev
Log:
Make clang_Cursor_getMangling don't mangle if the declaration isn't mangled

Right now clang_Cursor_getMangling will attempt to mangle any
declaration, even if the declaration isn't mangled (extern "C").  This
results in a partially mangled name which isn't useful for much. This
patch makes clang_Cursor_getMangling return an empty string if the
declaration isn't mangled.

Patch by Michael Wu .

Modified:
cfe/trunk/test/Index/print-mangled-name.cpp
cfe/trunk/tools/c-index-test/c-index-test.c
cfe/trunk/tools/libclang/CIndex.cpp

Modified: cfe/trunk/test/Index/print-mangled-name.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/print-mangled-name.cpp?rev=249437=249436=249437=diff
==
--- cfe/trunk/test/Index/print-mangled-name.cpp (original)
+++ cfe/trunk/test/Index/print-mangled-name.cpp Tue Oct  6 13:24:33 2015
@@ -29,3 +29,8 @@ int foo(S, S&);
 // ITANIUM: mangled=_Z3foo1SRS_
 // MACHO: mangled=__Z3foo1SRS_
 // MICROSOFT: mangled=?foo@@YAHUS
+
+extern "C" int foo(int);
+// ITANIUM: mangled=foo
+// MACHO: mangled=_foo
+// MICROSOFT: mangled=_foo

Modified: cfe/trunk/tools/c-index-test/c-index-test.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/c-index-test/c-index-test.c?rev=249437=249436=249437=diff
==
--- cfe/trunk/tools/c-index-test/c-index-test.c (original)
+++ cfe/trunk/tools/c-index-test/c-index-test.c Tue Oct  6 13:24:33 2015
@@ -1429,6 +1429,8 @@ static enum CXChildVisitResult PrintType
 
 static enum CXChildVisitResult PrintMangledName(CXCursor cursor, CXCursor p,
 CXClientData d) {
+  if (clang_isInvalid(clang_getCursorKind(cursor)))
+return CXChildVisit_Recurse;
   CXString MangledName;
   PrintCursor(cursor, NULL);
   MangledName = clang_Cursor_getMangling(cursor);

Modified: cfe/trunk/tools/libclang/CIndex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CIndex.cpp?rev=249437=249436=249437=diff
==
--- cfe/trunk/tools/libclang/CIndex.cpp (original)
+++ cfe/trunk/tools/libclang/CIndex.cpp Tue Oct  6 13:24:33 2015
@@ -3890,7 +3890,11 @@ CXString clang_Cursor_getMangling(CXCurs
 
   std::string FrontendBuf;
   llvm::raw_string_ostream FrontendBufOS(FrontendBuf);
-  MC->mangleName(ND, FrontendBufOS);
+  if (MC->shouldMangleDeclName(ND)) {
+MC->mangleName(ND, FrontendBufOS);
+  } else {
+ND->printName(FrontendBufOS);
+  }
 
   // Now apply backend mangling.
   std::unique_ptr DL(


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

Re: [PATCH] D13317: clang_Cursor_getMangling shouldn't mangle if the declaration isn't mangled

[Ehsan Akhgari via cfe-commits]

ehsan closed this revision.
ehsan added a comment.

Landed in r249437.


http://reviews.llvm.org/D13317



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

[libclc] r249445 - integer: remove explicit casts from _MIN definitions

[Aaron Watry via cfe-commits]

Author: awatry
Date: Tue Oct  6 14:12:12 2015
New Revision: 249445

URL: http://llvm.org/viewvc/llvm-project?rev=249445=rev
Log:
integer: remove explicit casts from _MIN definitions

The spec says (section 6.12.3, CL version 1.2):
  The macro names given in the following list must use the values
  specified. The values shall all be constant expressions suitable
  for use in #if preprocessing directives.

This commit addresses the second part of that statement.

Reviewed-by: Jan Vesely 
Reviewed-by: Tom Stellard 
CC: Moritz Pflanzer 
CC: Serge Martin 

Modified:
libclc/trunk/generic/include/clc/integer/definitions.h

Modified: libclc/trunk/generic/include/clc/integer/definitions.h
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/generic/include/clc/integer/definitions.h?rev=249445=249444=249445=diff
==
--- libclc/trunk/generic/include/clc/integer/definitions.h (original)
+++ libclc/trunk/generic/include/clc/integer/definitions.h Tue Oct  6 14:12:12 
2015
@@ -1,14 +1,14 @@
 #define CHAR_BIT 8
 #define INT_MAX 2147483647
-#define INT_MIN ((int)(-2147483647 - 1))
+#define INT_MIN (-2147483647 - 1)
 #define LONG_MAX  0x7fffL
 #define LONG_MIN (-0x7fffL - 1)
 #define CHAR_MAX SCHAR_MAX
 #define CHAR_MIN SCHAR_MIN
 #define SCHAR_MAX 127
-#define SCHAR_MIN ((char)(-127 - 1))
+#define SCHAR_MIN (-127 - 1)
 #define SHRT_MAX 32767
-#define SHRT_MIN ((short)(-32767 - 1))
+#define SHRT_MIN (-32767 - 1)
 #define UCHAR_MAX 255
 #define USHRT_MAX 65535
 #define UINT_MAX 0x


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

Re: [PATCH] D13317: clang_Cursor_getMangling shouldn't mangle if the declaration isn't mangled

[Michael Wu via cfe-commits]

michaelwu updated this revision to Diff 36645.
michaelwu added a comment.

Turns out clang_isUnexposed is what I wanted, rather than clang_isInvalid. I 
remember seeing clang_isInvalid work though..


http://reviews.llvm.org/D13317

Files:
  test/Index/print-mangled-name.cpp
  tools/c-index-test/c-index-test.c
  tools/libclang/CIndex.cpp

Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -3890,7 +3890,11 @@
 
   std::string FrontendBuf;
   llvm::raw_string_ostream FrontendBufOS(FrontendBuf);
-  MC->mangleName(ND, FrontendBufOS);
+  if (MC->shouldMangleDeclName(ND)) {
+MC->mangleName(ND, FrontendBufOS);
+  } else {
+ND->printName(FrontendBufOS);
+  }
 
   // Now apply backend mangling.
   std::unique_ptr DL(
Index: tools/c-index-test/c-index-test.c
===
--- tools/c-index-test/c-index-test.c
+++ tools/c-index-test/c-index-test.c
@@ -1429,6 +1429,8 @@
 
 static enum CXChildVisitResult PrintMangledName(CXCursor cursor, CXCursor p,
 CXClientData d) {
+  if (clang_isUnexposed(clang_getCursorKind(cursor)))
+return CXChildVisit_Recurse;
   CXString MangledName;
   PrintCursor(cursor, NULL);
   MangledName = clang_Cursor_getMangling(cursor);
Index: test/Index/print-mangled-name.cpp
===
--- test/Index/print-mangled-name.cpp
+++ test/Index/print-mangled-name.cpp
@@ -29,3 +29,8 @@
 // ITANIUM: mangled=_Z3foo1SRS_
 // MACHO: mangled=__Z3foo1SRS_
 // MICROSOFT: mangled=?foo@@YAHUS
+
+extern "C" int foo(int);
+// ITANIUM: mangled=foo
+// MACHO: mangled=_foo
+// MICROSOFT: mangled=_foo


Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -3890,7 +3890,11 @@
 
   std::string FrontendBuf;
   llvm::raw_string_ostream FrontendBufOS(FrontendBuf);
-  MC->mangleName(ND, FrontendBufOS);
+  if (MC->shouldMangleDeclName(ND)) {
+MC->mangleName(ND, FrontendBufOS);
+  } else {
+ND->printName(FrontendBufOS);
+  }
 
   // Now apply backend mangling.
   std::unique_ptr DL(
Index: tools/c-index-test/c-index-test.c
===
--- tools/c-index-test/c-index-test.c
+++ tools/c-index-test/c-index-test.c
@@ -1429,6 +1429,8 @@
 
 static enum CXChildVisitResult PrintMangledName(CXCursor cursor, CXCursor p,
 CXClientData d) {
+  if (clang_isUnexposed(clang_getCursorKind(cursor)))
+return CXChildVisit_Recurse;
   CXString MangledName;
   PrintCursor(cursor, NULL);
   MangledName = clang_Cursor_getMangling(cursor);
Index: test/Index/print-mangled-name.cpp
===
--- test/Index/print-mangled-name.cpp
+++ test/Index/print-mangled-name.cpp
@@ -29,3 +29,8 @@
 // ITANIUM: mangled=_Z3foo1SRS_
 // MACHO: mangled=__Z3foo1SRS_
 // MICROSOFT: mangled=?foo@@YAHUS
+
+extern "C" int foo(int);
+// ITANIUM: mangled=foo
+// MACHO: mangled=_foo
+// MICROSOFT: mangled=_foo
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r249440 - Revert r249437

[Ehsan Akhgari via cfe-commits]

Author: ehsan
Date: Tue Oct  6 13:53:12 2015
New Revision: 249440

URL: http://llvm.org/viewvc/llvm-project?rev=249440=rev
Log:
Revert r249437

Modified:
cfe/trunk/test/Index/print-mangled-name.cpp
cfe/trunk/tools/c-index-test/c-index-test.c
cfe/trunk/tools/libclang/CIndex.cpp

Modified: cfe/trunk/test/Index/print-mangled-name.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/print-mangled-name.cpp?rev=249440=249439=249440=diff
==
--- cfe/trunk/test/Index/print-mangled-name.cpp (original)
+++ cfe/trunk/test/Index/print-mangled-name.cpp Tue Oct  6 13:53:12 2015
@@ -29,8 +29,3 @@ int foo(S, S&);
 // ITANIUM: mangled=_Z3foo1SRS_
 // MACHO: mangled=__Z3foo1SRS_
 // MICROSOFT: mangled=?foo@@YAHUS
-
-extern "C" int foo(int);
-// ITANIUM: mangled=foo
-// MACHO: mangled=_foo
-// MICROSOFT: mangled=_foo

Modified: cfe/trunk/tools/c-index-test/c-index-test.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/c-index-test/c-index-test.c?rev=249440=249439=249440=diff
==
--- cfe/trunk/tools/c-index-test/c-index-test.c (original)
+++ cfe/trunk/tools/c-index-test/c-index-test.c Tue Oct  6 13:53:12 2015
@@ -1429,8 +1429,6 @@ static enum CXChildVisitResult PrintType
 
 static enum CXChildVisitResult PrintMangledName(CXCursor cursor, CXCursor p,
 CXClientData d) {
-  if (clang_isInvalid(clang_getCursorKind(cursor)))
-return CXChildVisit_Recurse;
   CXString MangledName;
   PrintCursor(cursor, NULL);
   MangledName = clang_Cursor_getMangling(cursor);

Modified: cfe/trunk/tools/libclang/CIndex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CIndex.cpp?rev=249440=249439=249440=diff
==
--- cfe/trunk/tools/libclang/CIndex.cpp (original)
+++ cfe/trunk/tools/libclang/CIndex.cpp Tue Oct  6 13:53:12 2015
@@ -3890,11 +3890,7 @@ CXString clang_Cursor_getMangling(CXCurs
 
   std::string FrontendBuf;
   llvm::raw_string_ostream FrontendBufOS(FrontendBuf);
-  if (MC->shouldMangleDeclName(ND)) {
-MC->mangleName(ND, FrontendBufOS);
-  } else {
-ND->printName(FrontendBufOS);
-  }
+  MC->mangleName(ND, FrontendBufOS);
 
   // Now apply backend mangling.
   std::unique_ptr DL(


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

[clang-tools-extra] r249444 - Change the write modes to "binary" so that line endings do not get munged on Windows. Otherwise, when this script is run, all files created on Windows have CRLF instead o

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Tue Oct  6 14:11:12 2015
New Revision: 249444

URL: http://llvm.org/viewvc/llvm-project?rev=249444=rev
Log:
Change the write modes to "binary" so that line endings do not get munged on 
Windows. Otherwise, when this script is run, all files created on Windows have 
CRLF instead of LF line endings.

Modified:
clang-tools-extra/trunk/clang-tidy/add_new_check.py

Modified: clang-tools-extra/trunk/clang-tidy/add_new_check.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/add_new_check.py?rev=249444=249443=249444=diff
==
--- clang-tools-extra/trunk/clang-tidy/add_new_check.py (original)
+++ clang-tools-extra/trunk/clang-tidy/add_new_check.py Tue Oct  6 14:11:12 2015
@@ -29,7 +29,7 @@ def adapt_cmake(module_path, check_name_
   return False
 
   print('Updating %s...' % filename)
-  with open(filename, 'w') as f:
+  with open(filename, 'wb') as f:
 cpp_found = False
 file_added = False
 for line in lines:
@@ -49,7 +49,7 @@ def write_header(module_path, module, ch
   check_name_dashes = module + '-' + check_name
   filename = os.path.join(module_path, check_name_camel) + '.h'
   print('Creating %s...' % filename)
-  with open(filename, 'w') as f:
+  with open(filename, 'wb') as f:
 header_guard = ('LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_' + module.upper() +
 '_' + check_name.upper().replace('-', '_') + '_H')
 f.write('//===--- ')
@@ -100,7 +100,7 @@ public:
 def write_implementation(module_path, check_name_camel):
   filename = os.path.join(module_path, check_name_camel) + '.cpp'
   print('Creating %s...' % filename)
-  with open(filename, 'w') as f:
+  with open(filename, 'wb') as f:
 f.write('//===--- ')
 f.write(os.path.basename(filename))
 f.write(' - clang-tidy')
@@ -153,7 +153,7 @@ def adapt_module(module_path, module, ch
 lines = f.readlines()
 
   print('Updating %s...' % filename)
-  with open(filename, 'w') as f:
+  with open(filename, 'wb') as f:
 header_added = False
 header_found = False
 check_added = False
@@ -191,7 +191,7 @@ def write_test(module_path, module, chec
   os.path.join(module_path, '../../test/clang-tidy',
check_name_dashes + '.cpp'))
   print('Creating %s...' % filename)
-  with open(filename, 'w') as f:
+  with open(filename, 'wb') as f:
 f.write(
 """// RUN: %%python %%S/check_clang_tidy.py %%s %(check_name_dashes)s %%t
 
@@ -222,7 +222,7 @@ def update_checks_list(module_path):
   checks.sort()
 
   print('Updating %s...' % filename)
-  with open(filename, 'w') as f:
+  with open(filename, 'wb') as f:
 for line in lines:
   f.write(line)
   if line.startswith('.. toctree::'):
@@ -236,7 +236,7 @@ def write_docs(module_path, module, chec
   os.path.join(module_path, '../../docs/clang-tidy/checks/',
check_name_dashes + '.rst'))
   print('Creating %s...' % filename)
-  with open(filename, 'w') as f:
+  with open(filename, 'wb') as f:
 f.write(
 """%(check_name_dashes)s
 %(underline)s


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

Re: [PATCH] D13326: [PGO]: Eliminate __llvm_profile_register calls for Linux (clang changes)

[Justin Bogner via cfe-commits]

David Li  writes:
> davidxl updated this revision to Diff 36316.
> davidxl added a comment.
>
> I have modified the implementation to not use linker script, so this
> clang patch becomes strictly refactoring with NFC. I think it is still
> a good thing to have this in so that similar tunings like this can be
> done in the future.

Sure. I have a couple of nitpicks but this basically LGTM.

>
> http://reviews.llvm.org/D13326
>
> Files:
>   include/clang/Driver/ToolChain.h
>   lib/Driver/SanitizerArgs.cpp
>   lib/Driver/ToolChain.cpp
>   lib/Driver/ToolChains.cpp
>   lib/Driver/ToolChains.h
>   lib/Driver/Tools.cpp
>
> Index: lib/Driver/Tools.cpp
> ===
> --- lib/Driver/Tools.cpp
> +++ lib/Driver/Tools.cpp
> @@ -2402,83 +2402,12 @@
>}
>  }
>  
> -// Until ARM libraries are build separately, we have them all in one library
> -static StringRef getArchNameForCompilerRTLib(const ToolChain ,
> - const ArgList ) {
> -  const llvm::Triple  = TC.getTriple();
> -  bool IsWindows = Triple.isOSWindows();
> -
> -  if (Triple.isWindowsMSVCEnvironment() && TC.getArch() == llvm::Triple::x86)
> -return "i386";
> -
> -  if (TC.getArch() == llvm::Triple::arm || TC.getArch() == 
> llvm::Triple::armeb)
> -return (arm::getARMFloatABI(TC, Args) == arm::FloatABI::Hard && 
> !IsWindows)
> -   ? "armhf"
> -   : "arm";
> -
> -  return TC.getArchName();
> -}
> -
> -static SmallString<128> getCompilerRTLibDir(const ToolChain ) {
> -  // The runtimes are located in the OS-specific resource directory.
> -  SmallString<128> Res(TC.getDriver().ResourceDir);
> -  const llvm::Triple  = TC.getTriple();
> -  // TC.getOS() yield "freebsd10.0" whereas "freebsd" is expected.
> -  StringRef OSLibName =
> -  (Triple.getOS() == llvm::Triple::FreeBSD) ? "freebsd" : TC.getOS();
> -  llvm::sys::path::append(Res, "lib", OSLibName);
> -  return Res;
> -}
> -
> -SmallString<128> tools::getCompilerRT(const ToolChain , const ArgList 
> ,
> -  StringRef Component, bool Shared) {
> -  const char *Env = TC.getTriple().getEnvironment() == llvm::Triple::Android
> -? "-android"
> -: "";
> -
> -  bool IsOSWindows = TC.getTriple().isOSWindows();
> -  bool IsITANMSVCWindows = TC.getTriple().isWindowsMSVCEnvironment() ||
> -   TC.getTriple().isWindowsItaniumEnvironment();
> -  StringRef Arch = getArchNameForCompilerRTLib(TC, Args);
> -  const char *Prefix = IsITANMSVCWindows ? "" : "lib";
> -  const char *Suffix =
> -  Shared ? (IsOSWindows ? ".dll" : ".so") : (IsITANMSVCWindows ? ".lib" 
> : ".a");
> -
> -  SmallString<128> Path = getCompilerRTLibDir(TC);
> -  llvm::sys::path::append(Path, Prefix + Twine("clang_rt.") + Component + 
> "-" +
> -Arch + Env + Suffix);
> -
> -  return Path;
> -}
> -
> -static const char *getCompilerRTArgString(const ToolChain ,
> -  const llvm::opt::ArgList ,
> -  StringRef Component,
> -  bool Shared = false) {
> -  return Args.MakeArgString(getCompilerRT(TC, Args, Component, Shared));
> -}
> -
>  // This adds the static libclang_rt.builtins-arch.a directly to the command 
> line
>  // FIXME: Make sure we can also emit shared objects if they're requested
>  // and available, check for possible errors, etc.
>  static void addClangRT(const ToolChain , const ArgList ,
> ArgStringList ) {
> -  CmdArgs.push_back(getCompilerRTArgString(TC, Args, "builtins"));
> -}
> -
> -static void addProfileRT(const ToolChain , const ArgList ,
> - ArgStringList ) {
> -  if (!(Args.hasFlag(options::OPT_fprofile_arcs, 
> options::OPT_fno_profile_arcs,
> - false) ||
> -Args.hasArg(options::OPT_fprofile_generate) ||
> -Args.hasArg(options::OPT_fprofile_generate_EQ) ||
> -Args.hasArg(options::OPT_fprofile_instr_generate) ||
> -Args.hasArg(options::OPT_fprofile_instr_generate_EQ) ||
> -Args.hasArg(options::OPT_fcreate_profile) ||
> -Args.hasArg(options::OPT_coverage)))
> -return;
> -
> -  CmdArgs.push_back(getCompilerRTArgString(TC, Args, "profile"));
> +  CmdArgs.push_back(TC.getCompilerRTArgString(Args, "builtins"));
>  }
>  
>  namespace {
> @@ -2559,7 +2488,7 @@
>// whole-archive.
>if (!IsShared)
>  CmdArgs.push_back("-whole-archive");
> -  CmdArgs.push_back(getCompilerRTArgString(TC, Args, Sanitizer, IsShared));
> +  CmdArgs.push_back(TC.getCompilerRTArgString(Args, Sanitizer, IsShared));
>if (!IsShared)
>  CmdArgs.push_back("-no-whole-archive");
>  }
> @@ -2569,7 +2498,7 @@
>  static bool addSanitizerDynamicList(const ToolChain , const ArgList ,
>  

Re: [PATCH] D13407: [libcxx] Capture configuration information when installing the libc++ headers

[Eric Fiselier via cfe-commits]

EricWF added inline comments.


Comment at: include/CMakeLists.txt:31
@@ +30,3 @@
+# by  prepending __config_site to the current __config header.
+# TODO(EricWF) Is it portable to use "cat" and ">>"?
+add_custom_command(OUTPUT ${LIBCXX_BINARY_DIR}/__generated_config

eugenis wrote:
> I don't think >> would work on windows.
> Do you really need __generated_config to be created at build time (as opposed 
> to configure time)? You could use file(read) and file(append) then.
> 
I would strongly prefer the file got generated at build time so that it 
contains any changes made in the source tree. Any other behavior is developer 
hostile and non-obvious. In order to keep the installed headers consistent we 
need to do this. Although I hope there is a better way to achieve this.


http://reviews.llvm.org/D13407



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

Re: r249413 - [Tooling] Don't run a tool invocation without a FileManager.

[David Blaikie via cfe-commits]

On Tue, Oct 6, 2015 at 8:04 AM, Benjamin Kramer via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: d0k
> Date: Tue Oct  6 10:04:13 2015
> New Revision: 249413
>
> URL: http://llvm.org/viewvc/llvm-project?rev=249413=rev
> Log:
> [Tooling] Don't run a tool invocation without a FileManager.
>
> Fixes a crash regression from r249410.
>
> Modified:
> cfe/trunk/lib/Tooling/Tooling.cpp
>
> Modified: cfe/trunk/lib/Tooling/Tooling.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Tooling.cpp?rev=249413=249412=249413=diff
>
> ==
> --- cfe/trunk/lib/Tooling/Tooling.cpp (original)
> +++ cfe/trunk/lib/Tooling/Tooling.cpp Tue Oct  6 10:04:13 2015
> @@ -455,8 +455,10 @@ std::unique_ptr buildASTFromCod
>
>std::vector ASTs;
>ASTBuilderAction Action(ASTs);
> +  llvm::IntrusiveRefCntPtr Files(
> +  new FileManager(FileSystemOptions()));
>ToolInvocation Invocation(getSyntaxOnlyToolArgs(Args, FileNameRef),
> ,
> -nullptr, PCHContainerOps);
> +Files.get(), PCHContainerOps);
>

Why is this a pointer parameter instead of a reference?


>
>SmallString<1024> CodeStorage;
>Invocation.mapVirtualFile(FileNameRef,
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13407: [libcxx] Capture configuration information when installing the libc++ headers

[Evgeniy Stepanov via cfe-commits]

eugenis added a comment.

Looks great!



Comment at: include/CMakeLists.txt:31
@@ +30,3 @@
+# by  prepending __config_site to the current __config header.
+# TODO(EricWF) Is it portable to use "cat" and ">>"?
+add_custom_command(OUTPUT ${LIBCXX_BINARY_DIR}/__generated_config

I don't think >> would work on windows.
Do you really need __generated_config to be created at build time (as opposed 
to configure time)? You could use file(read) and file(append) then.



http://reviews.llvm.org/D13407



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

Re: [PATCH] D13446: [PATCH] Add checker discouraging definition of variadic function definitions in C++

[Joerg Sonnenberger via cfe-commits]

On Mon, Oct 05, 2015 at 07:36:08PM +, Aaron Ballman via cfe-commits wrote:
> C-style variadic functions (using an ellipsis) can be dangerous in C++
> due to the inherit lack of type safety with argument passing. Better
> alternatives exist, such as function currying (like STL stream objects
> use), or function parameter packs. This patch adds a checker to
> diagnose definitions of variadic functions in C++ code, but still
> allows variadic function declarations, as those can be safely used to
> good effect for SFINAE patterns.

I would restrict this a bit to exclude function definitions with C
linkage. If you have such a ABI requirement, you normally can't replace
it with any of the alternatives.

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

[libcxx] r249461 - Updated issue 2476

[Marshall Clow via cfe-commits]

Author: marshall
Date: Tue Oct  6 15:35:15 2015
New Revision: 249461

URL: http://llvm.org/viewvc/llvm-project?rev=249461=rev
Log:
Updated issue 2476

Modified:
libcxx/trunk/www/kona.html

Modified: libcxx/trunk/www/kona.html
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/www/kona.html?rev=249461=249460=249461=diff
==
--- libcxx/trunk/www/kona.html (original)
+++ libcxx/trunk/www/kona.html Tue Oct  6 15:35:15 2015
@@ -84,7 +84,7 @@
http://cplusplus.github.io/LWG/lwg-defects.html#2466;>2466allocator_traits::max_size()
 default behavior is incorrectKonaPatch Ready
http://cplusplus.github.io/LWG/lwg-defects.html#2469;>2469Wrong
 specification of Requires clause of operator[] for map and 
unordered_mapKona
http://cplusplus.github.io/LWG/lwg-defects.html#2473;>2473basic_filebuf's
 relation to C FILE semanticsKonaComplete
-   http://cplusplus.github.io/LWG/lwg-defects.html#2476;>2476scoped_allocator_adaptor
 is not assignableKona
+   http://cplusplus.github.io/LWG/lwg-defects.html#2476;>2476scoped_allocator_adaptor
 is not assignableKonaPatch Ready
http://cplusplus.github.io/LWG/lwg-defects.html#2477;>2477Inconsistency
 of wordings in std::vector::erase() and 
std::deque::erase()Kona
http://cplusplus.github.io/LWG/lwg-defects.html#2483;>2483throw_with_nested()
 should use is_finalKonaComplete
http://cplusplus.github.io/LWG/lwg-defects.html#2484;>2484rethrow_if_nested()
 is doubly unimplementableKonaComplete
@@ -129,7 +129,7 @@
 2466 - Simple change; need a test. Patch Available
 2469 - This is a followon to 2464, which we're done. This restates a bunch 
of operations in terms of newer ops. Probably refactoring to do here, but tests 
shouldn't change.
 2473 - There are two changes here; one to filebuf::seekpos and 
the other to filebuf::sync. We do both of these already.
-2476 - Simple change; need tests.
+2476 - Just needed tests. Patch Available
 2477 - Definitely wording cleanup, but check the tests.
 2483 - We already do this.
 2484 - We already do this.


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

Re: [PATCH] D13446: [PATCH] Add checker discouraging definition of variadic function definitions in C++

[Joerg Sonnenberger via cfe-commits]

On Tue, Oct 06, 2015 at 04:20:05PM -0400, Aaron Ballman via cfe-commits wrote:
> On Tue, Oct 6, 2015 at 4:12 PM, Joerg Sonnenberger via cfe-commits
>  wrote:
> > On Mon, Oct 05, 2015 at 07:36:08PM +, Aaron Ballman via cfe-commits 
> > wrote:
> >> C-style variadic functions (using an ellipsis) can be dangerous in C++
> >> due to the inherit lack of type safety with argument passing. Better
> >> alternatives exist, such as function currying (like STL stream objects
> >> use), or function parameter packs. This patch adds a checker to
> >> diagnose definitions of variadic functions in C++ code, but still
> >> allows variadic function declarations, as those can be safely used to
> >> good effect for SFINAE patterns.
> >
> > I would restrict this a bit to exclude function definitions with C
> > linkage. If you have such a ABI requirement, you normally can't replace
> > it with any of the alternatives.
> 
> Under what circumstances would you run into this issue with C++ code?
> The only circumstance I can think of is if you have a C API that takes
> a function pointer to a varargs function as an argument. Is that the
> situation you are thinking of, or are there others?

Consider a stdio implementation written in C++? Wouldn't the
implementation of e.g. vprintf trigger exactly this warning?

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

Re: [PATCH] D13446: [PATCH] Add checker discouraging definition of variadic function definitions in C++

[Aaron Ballman via cfe-commits]

On Tue, Oct 6, 2015 at 4:49 PM, Joerg Sonnenberger via cfe-commits
 wrote:
> On Tue, Oct 06, 2015 at 04:20:05PM -0400, Aaron Ballman via cfe-commits wrote:
>> On Tue, Oct 6, 2015 at 4:12 PM, Joerg Sonnenberger via cfe-commits
>>  wrote:
>> > On Mon, Oct 05, 2015 at 07:36:08PM +, Aaron Ballman via cfe-commits 
>> > wrote:
>> >> C-style variadic functions (using an ellipsis) can be dangerous in C++
>> >> due to the inherit lack of type safety with argument passing. Better
>> >> alternatives exist, such as function currying (like STL stream objects
>> >> use), or function parameter packs. This patch adds a checker to
>> >> diagnose definitions of variadic functions in C++ code, but still
>> >> allows variadic function declarations, as those can be safely used to
>> >> good effect for SFINAE patterns.
>> >
>> > I would restrict this a bit to exclude function definitions with C
>> > linkage. If you have such a ABI requirement, you normally can't replace
>> > it with any of the alternatives.
>>
>> Under what circumstances would you run into this issue with C++ code?
>> The only circumstance I can think of is if you have a C API that takes
>> a function pointer to a varargs function as an argument. Is that the
>> situation you are thinking of, or are there others?
>
> Consider a stdio implementation written in C++? Wouldn't the
> implementation of e.g. vprintf trigger exactly this warning?

It would, but it would also trigger a *lot* of warnings that aren't
directed at implementers as well, such as anything about relying on
implementation-defined or undefined behavior (like the implementation
of offsetof(), as a trivial example). It's a good point to keep in
mind though.

Thanks!

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

Re: [libclc] r249445 - integer: remove explicit casts from _MIN definitions

[Joerg Sonnenberger via cfe-commits]

On Tue, Oct 06, 2015 at 07:12:12PM -, Aaron Watry via cfe-commits wrote:
> Author: awatry
> Date: Tue Oct  6 14:12:12 2015
> New Revision: 249445
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=249445=rev
> Log:
> integer: remove explicit casts from _MIN definitions

Why do this definitions exist in first place? Can't they use the CPP
macros already defined by the compiler?

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

Re: [PATCH] D13446: [PATCH] Add checker discouraging definition of variadic function definitions in C++

[Aaron Ballman via cfe-commits]

On Tue, Oct 6, 2015 at 4:12 PM, Joerg Sonnenberger via cfe-commits
 wrote:
> On Mon, Oct 05, 2015 at 07:36:08PM +, Aaron Ballman via cfe-commits wrote:
>> C-style variadic functions (using an ellipsis) can be dangerous in C++
>> due to the inherit lack of type safety with argument passing. Better
>> alternatives exist, such as function currying (like STL stream objects
>> use), or function parameter packs. This patch adds a checker to
>> diagnose definitions of variadic functions in C++ code, but still
>> allows variadic function declarations, as those can be safely used to
>> good effect for SFINAE patterns.
>
> I would restrict this a bit to exclude function definitions with C
> linkage. If you have such a ABI requirement, you normally can't replace
> it with any of the alternatives.

Under what circumstances would you run into this issue with C++ code?
The only circumstance I can think of is if you have a C API that takes
a function pointer to a varargs function as an argument. Is that the
situation you are thinking of, or are there others?

Thank you for the feedback!

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

[libcxx] r249458 - Our test allocators support move/copy construction; they should support move/copy assignment as well

[Marshall Clow via cfe-commits]

Author: marshall
Date: Tue Oct  6 15:30:56 2015
New Revision: 249458

URL: http://llvm.org/viewvc/llvm-project?rev=249458=rev
Log:
Our test allocators support move/copy construction; they should support 
move/copy assignment as well

Modified:
libcxx/trunk/test/support/allocators.h

Modified: libcxx/trunk/test/support/allocators.h
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/support/allocators.h?rev=249458=249457=249458=diff
==
--- libcxx/trunk/test/support/allocators.h (original)
+++ libcxx/trunk/test/support/allocators.h Tue Oct  6 15:30:56 2015
@@ -35,6 +35,8 @@ public:
 
 A1(const A1& a) TEST_NOEXCEPT : id_(a.id()) {copy_called = true;}
 A1(A1&& a)  TEST_NOEXCEPT : id_(a.id()) {move_called = true;}
+A1& operator=(const A1& a) TEST_NOEXCEPT { id_ = a.id(); copy_called = 
true; return *this;}
+A1& operator=(A1&& a)  TEST_NOEXCEPT { id_ = a.id(); move_called = 
true; return *this;}
 
 template 
 A1(const A1& a) TEST_NOEXCEPT : id_(a.id()) {copy_called = true;}
@@ -96,6 +98,8 @@ public:
 
 A2(const A2& a) TEST_NOEXCEPT : id_(a.id()) {copy_called = true;}
 A2(A2&& a)  TEST_NOEXCEPT : id_(a.id()) {move_called = true;}
+A2& operator=(const A2& a) TEST_NOEXCEPT { id_ = a.id(); copy_called = 
true; return *this;}
+A2& operator=(A2&& a)  TEST_NOEXCEPT { id_ = a.id(); move_called = 
true; return *this;}
 
 T* allocate(std::size_t n, const void* hint)
 {
@@ -142,7 +146,9 @@ public:
 static bool destroy_called;
 
 A3(const A3& a) TEST_NOEXCEPT : id_(a.id()) {copy_called = true;}
-A3(A3&& a)  TEST_NOEXCEPT: id_(a.id())  {move_called = true;}
+A3(A3&& a)  TEST_NOEXCEPT : id_(a.id())  {move_called = true;}
+A3& operator=(const A3& a) TEST_NOEXCEPT { id_ = a.id(); copy_called = 
true; return *this;}
+A3& operator=(A3&& a)  TEST_NOEXCEPT { id_ = a.id(); move_called = 
true; return *this;}
 
 template 
 void construct(U* p, Args&& ...args)


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

Re: [PATCH] D13407: [libcxx] Capture configuration information when installing the libc++ headers

[Eric Fiselier via cfe-commits]

EricWF added inline comments.


Comment at: include/CMakeLists.txt:31
@@ +30,3 @@
+# by  prepending __config_site to the current __config header.
+# TODO(EricWF) Is it portable to use "cat" and ">>"?
+add_custom_command(OUTPUT ${LIBCXX_BINARY_DIR}/__generated_config

eugenis wrote:
> EricWF wrote:
> > eugenis wrote:
> > > I don't think >> would work on windows.
> > > Do you really need __generated_config to be created at build time (as 
> > > opposed to configure time)? You could use file(read) and file(append) 
> > > then.
> > > 
> > I would strongly prefer the file got generated at build time so that it 
> > contains any changes made in the source tree. Any other behavior is 
> > developer hostile and non-obvious. In order to keep the installed headers 
> > consistent we need to do this. Although I hope there is a better way to 
> > achieve this.
> Right, good point. Then you could go back to the approach in D11963 where you 
> called cmake in a custom command with a small script that used file(*) 
> commands.
> 
That approach had a problem because the cmake INSTALL command used to invoke 
the script doesn't take a "COMPONENT" argument which would break the 
"install-libcxx" rule. On windows we could do it with a call out to python or a 
shell script. Would that work?


http://reviews.llvm.org/D13407



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

Re: [PATCH] D13407: [libcxx] Capture configuration information when installing the libc++ headers

[Reid Kleckner via cfe-commits]

rnk added a subscriber: rnk.


Comment at: include/CMakeLists.txt:31
@@ +30,3 @@
+# by  prepending __config_site to the current __config header.
+# TODO(EricWF) Is it portable to use "cat" and ">>"?
+add_custom_command(OUTPUT ${LIBCXX_BINARY_DIR}/__generated_config

EricWF wrote:
> eugenis wrote:
> > EricWF wrote:
> > > eugenis wrote:
> > > > I don't think >> would work on windows.
> > > > Do you really need __generated_config to be created at build time (as 
> > > > opposed to configure time)? You could use file(read) and file(append) 
> > > > then.
> > > > 
> > > I would strongly prefer the file got generated at build time so that it 
> > > contains any changes made in the source tree. Any other behavior is 
> > > developer hostile and non-obvious. In order to keep the installed headers 
> > > consistent we need to do this. Although I hope there is a better way to 
> > > achieve this.
> > Right, good point. Then you could go back to the approach in D11963 where 
> > you called cmake in a custom command with a small script that used file(*) 
> > commands.
> > 
> That approach had a problem because the cmake INSTALL command used to invoke 
> the script doesn't take a "COMPONENT" argument which would break the 
> "install-libcxx" rule. On windows we could do it with a call out to python or 
> a shell script. Would that work?
The >> operator should work on Windows. It's supported by cmd. However, cat 
generally isn't available. If you use 'type' in place of 'cat' it should work.


http://reviews.llvm.org/D13407



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

Re: [PATCH] D13311: [clang-tidy] Add check cppcoreguidelines-pro-bounds-pointer-arithmetic

[Matthias Gehre via cfe-commits]

mgehre marked 2 inline comments as done.


Comment at: 
test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic.cpp:37
@@ +36,3 @@
+  q -= i;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use pointer arithmetic
+  q -= ENUM_LITERAL;

aaron.ballman wrote:
> I don't think this comment is "done" yet. I still don't know how this check 
> is intended to handle code like that. Does it currently diagnose? Does it not 
> diagnose? Should it diagnose?
I had added your code, see line 55 of this file.
It should diagnose, because an arithmetic operation (+) is used, which results 
in a (possibly) changed pointer.
It does diagnose, as the CHECK-MESSAGE shows.


Comment at: 
test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic.cpp:51
@@ +50,3 @@
+
+  i = p[1];
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use pointer arithmetic

aaron.ballman wrote:
> How well does this handle code like:
> ```
> void f(int i[], size_t s) {
>   i[s - 1] = 0;
> }
> ```
> Does it diagnose, and should it?
Good point, I'll ask at https://github.com/isocpp/CppCoreGuidelines/issues/299


http://reviews.llvm.org/D13311



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

Re: [PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

[Matthias Gehre via cfe-commits]

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

I cannot think of any way to improve the usefulness of the diagnostic in the 
non-polymorphic case.
For now, this patch has a link to the CppCoreGuidelines document in the 
docs/**/*.rst file.

Also, this check is not enabled by default. One has to explicitly enable it, 
and may at that
point reason about the usefulness of the CppCoreGuidelines and this check.



Comment at: 
test/clang-tidy/cppcoreguidelines-pro-type-static-cast-downcast.cpp:39
@@ +38,3 @@
+  auto PP0 = static_cast(new PolymorphicBase());
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: do not use static_cast to 
downcast from a base to a derived class; use dynamic_cast instead 
[cppcoreguidelines-pro-type-static-cast-downcast]
+  // CHECK-FIXES: auto PP0 = dynamic_cast(new 
PolymorphicBase());

aaron.ballman wrote:
> No need to have the [cppcoreguidelines-pro-type-static-cast-downcast] part of 
> the message on anything but the first diagnostic in the file. (This reduces 
> clutter, but we test it one time just to make sure it's there).
I can remove it on the remaining messages that end in "use dynamic_cast 
instead". I need to keep it in the non-polymorphic case
to make sure that they are not followed by "use dynamic_cast instead".


http://reviews.llvm.org/D13368



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

[libcxx] r249475 - Remove unnecessary inline functions capturing the contents of C library macros.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Tue Oct  6 17:03:22 2015
New Revision: 249475

URL: http://llvm.org/viewvc/llvm-project?rev=249475=rev
Log:
Remove unnecessary inline functions capturing the contents of C library macros.

The C standard requires that these be provided as functions even if they're
also provided as macros, and a strict reading of the C++ standard library rules
suggests that (for instance) &::isdigit == &::std::isdigit, so these wrappers
are technically non-conforming.

Modified:
libcxx/trunk/include/cctype
libcxx/trunk/include/cstdio
libcxx/trunk/include/cwctype

Modified: libcxx/trunk/include/cctype
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/cctype?rev=249475=249474=249475=diff
==
--- libcxx/trunk/include/cctype (original)
+++ libcxx/trunk/include/cctype Tue Oct  6 17:03:22 2015
@@ -48,117 +48,34 @@ int toupper(int c);
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-#ifdef isalnum
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_isalnum(int __c) {return 
isalnum(__c);}
 #undef isalnum
-inline _LIBCPP_INLINE_VISIBILITY int isalnum(int __c) {return 
__libcpp_isalnum(__c);}
-#else  // isalnum
 using ::isalnum;
-#endif  // isalnum
-
-#ifdef isalpha
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_isalpha(int __c) {return 
isalpha(__c);}
 #undef isalpha
-inline _LIBCPP_INLINE_VISIBILITY int isalpha(int __c) {return 
__libcpp_isalpha(__c);}
-#else  // isalpha
 using ::isalpha;
-#endif  // isalpha
-
-#ifdef isblank
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_isblank(int __c) {return 
isblank(__c);}
 #undef isblank
-inline _LIBCPP_INLINE_VISIBILITY int isblank(int __c) {return 
__libcpp_isblank(__c);}
-#else  // isblank
 using ::isblank;
-#endif  // isblank
-
-#ifdef iscntrl
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_iscntrl(int __c) {return 
iscntrl(__c);}
 #undef iscntrl
-inline _LIBCPP_INLINE_VISIBILITY int iscntrl(int __c) {return 
__libcpp_iscntrl(__c);}
-#else  // iscntrl
 using ::iscntrl;
-#endif  // iscntrl
-
-#ifdef isdigit
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_isdigit(int __c) {return 
isdigit(__c);}
 #undef isdigit
-inline _LIBCPP_INLINE_VISIBILITY int isdigit(int __c) {return 
__libcpp_isdigit(__c);}
-#else  // isdigit
 using ::isdigit;
-#endif  // isdigit
-
-#ifdef isgraph
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_isgraph(int __c) {return 
isgraph(__c);}
 #undef isgraph
-inline _LIBCPP_INLINE_VISIBILITY int isgraph(int __c) {return 
__libcpp_isgraph(__c);}
-#else  // isgraph
 using ::isgraph;
-#endif  // isgraph
-
-#ifdef islower
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_islower(int __c) {return 
islower(__c);}
 #undef islower
-inline _LIBCPP_INLINE_VISIBILITY int islower(int __c) {return 
__libcpp_islower(__c);}
-#else  // islower
 using ::islower;
-#endif  // islower
-
-#ifdef isprint
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_isprint(int __c) {return 
isprint(__c);}
 #undef isprint
-inline _LIBCPP_INLINE_VISIBILITY int isprint(int __c) {return 
__libcpp_isprint(__c);}
-#else  // isprint
 using ::isprint;
-#endif  // isprint
-
-#ifdef ispunct
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_ispunct(int __c) {return 
ispunct(__c);}
 #undef ispunct
-inline _LIBCPP_INLINE_VISIBILITY int ispunct(int __c) {return 
__libcpp_ispunct(__c);}
-#else  // ispunct
 using ::ispunct;
-#endif  // ispunct
-
-#ifdef isspace
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_isspace(int __c) {return 
isspace(__c);}
 #undef isspace
-inline _LIBCPP_INLINE_VISIBILITY int isspace(int __c) {return 
__libcpp_isspace(__c);}
-#else  // isspace
 using ::isspace;
-#endif  // isspace
-
-#ifdef isupper
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_isupper(int __c) {return 
isupper(__c);}
 #undef isupper
-inline _LIBCPP_INLINE_VISIBILITY int isupper(int __c) {return 
__libcpp_isupper(__c);}
-#else  // isupper
 using ::isupper;
-#endif  // isupper
-
-#ifdef isxdigit
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_isxdigit(int __c) {return 
isxdigit(__c);}
 #undef isxdigit
-inline _LIBCPP_INLINE_VISIBILITY int isxdigit(int __c) {return 
__libcpp_isxdigit(__c);}
-#else  // isxdigit
 using ::isxdigit;
-#endif  // isxdigit
-
-#ifdef tolower
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_tolower(int __c) {return 
tolower(__c);}
 #undef tolower
-inline _LIBCPP_INLINE_VISIBILITY int tolower(int __c) {return 
__libcpp_tolower(__c);}
-#else  // tolower
 using ::tolower;
-#endif  // tolower
-
-#ifdef toupper
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_toupper(int __c) {return 
toupper(__c);}
 #undef toupper
-inline _LIBCPP_INLINE_VISIBILITY int toupper(int __c) {return 
__libcpp_toupper(__c);}
-#else  // toupper
 using ::toupper;
-#endif  // toupper
 
 _LIBCPP_END_NAMESPACE_STD
 

Modified: libcxx/trunk/include/cstdio
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/cstdio?rev=249475=249474=249475=diff
==
--- 

Re: [PATCH] D12747: Implement [depr.c.headers]

[Richard Smith via cfe-commits]

Split  header out of 

On Tue, Oct 6, 2015 at 3:07 PM, Richard Smith  wrote:

> Next: factoring the definition of std::nullptr_t out into a separate file,
> so that  and  can both use it, without 
> including  and without  providing a ::nullptr_t like
>  does.
>
> On Tue, Oct 6, 2015 at 3:02 PM, Eric Fiselier  wrote:
>
>> LGTM.
>>
>> On Tue, Oct 6, 2015 at 3:58 PM, Richard Smith 
>> wrote:
>> > On Mon, Oct 5, 2015 at 7:10 PM, Eric Fiselier  wrote:
>> >>
>> >> EricWF added a comment.
>> >>
>> >> I think thing change will help us close a number out outstanding bugs.
>> I
>> >> don't have any fundamental objections to this approach.  However the
>> size of
>> >> this patch scares me.  I understand the changes are mostly mechanical
>> but
>> >> their size can hide things. For example has anybody noticed that your
>> >> internal changes to `` are in this patch? It would be nice to
>> break
>> >> this down into more digestible pieces that could be quickly spot
>> checked.
>> >
>> >
>> > OK. First such patch is attached. It just removes the macro-capturing
>> > wrapper functions.
>>
>
>
diff --git include/cctype include/cctype
index db16343..a68c2a0 100644
--- include/cctype
+++ include/cctype
@@ -37,10 +37,6 @@ int toupper(int c);
 
 #include <__config>
 #include 
-#if defined(_LIBCPP_MSVCRT)
-#include "support/win32/support.h"
-#include "support/win32/locale_win32.h"
-#endif // _LIBCPP_MSVCRT
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
@@ -48,33 +44,19 @@ int toupper(int c);
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-#undef isalnum
 using ::isalnum;
-#undef isalpha
 using ::isalpha;
-#undef isblank
 using ::isblank;
-#undef iscntrl
 using ::iscntrl;
-#undef isdigit
 using ::isdigit;
-#undef isgraph
 using ::isgraph;
-#undef islower
 using ::islower;
-#undef isprint
 using ::isprint;
-#undef ispunct
 using ::ispunct;
-#undef isspace
 using ::isspace;
-#undef isupper
 using ::isupper;
-#undef isxdigit
 using ::isxdigit;
-#undef tolower
 using ::tolower;
-#undef toupper
 using ::toupper;
 
 _LIBCPP_END_NAMESPACE_STD
diff --git include/ctype.h include/ctype.h
new file mode 100644
index 000..63f0b29
--- /dev/null
+++ include/ctype.h
@@ -0,0 +1,68 @@
+// -*- C++ -*-
+//=== ctype.h 
-===//
+//
+// 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_CTYPE_H
+#define _LIBCPP_CTYPE_H
+
+/*
+ctype.h synopsis
+
+int isalnum(int c);
+int isalpha(int c);
+int isblank(int c);  // C99
+int iscntrl(int c);
+int isdigit(int c);
+int isgraph(int c);
+int islower(int c);
+int isprint(int c);
+int ispunct(int c);
+int isspace(int c);
+int isupper(int c);
+int isxdigit(int c);
+int tolower(int c);
+int toupper(int c);
+*/
+
+#include <__config>
+#include_next 
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#pragma GCC system_header
+#endif
+
+#ifdef __cplusplus
+
+#if defined(_LIBCPP_MSVCRT)
+// We support including .h headers inside 'extern "C"' contexts, so switch
+// back to C++ linkage before including these C++ headers.
+extern "C++" {
+  #include "support/win32/support.h"
+  #include "support/win32/locale_win32.h"
+}
+#endif // _LIBCPP_MSVCRT
+
+#undef isalnum
+#undef isalpha
+#undef isblank
+#undef iscntrl
+#undef isdigit
+#undef isgraph
+#undef islower
+#undef isprint
+#undef ispunct
+#undef isspace
+#undef isupper
+#undef isxdigit
+#undef tolower
+#undef toupper
+
+#endif
+
+#endif  // _LIBCPP_CTYPE_H
diff --git test/std/strings/c.strings/cctype.pass.cpp 
test/std/strings/c.strings/cctype.pass.cpp
index 867338f..027fbcd 100644
--- test/std/strings/c.strings/cctype.pass.cpp
+++ test/std/strings/c.strings/cctype.pass.cpp
@@ -86,18 +86,18 @@ int main()
 static_assert((std::is_same::value), "");
 static_assert((std::is_same::value), "");
 
-assert(isalnum('a'));
-assert(isalpha('a'));
-assert(isblank(' '));
-assert(!iscntrl(' '));
-assert(!isdigit('a'));
-assert(isgraph('a'));
-assert(islower('a'));
-assert(isprint('a'));
-assert(!ispunct('a'));
-assert(!isspace('a'));
-assert(!isupper('a'));
-assert(isxdigit('a'));
-assert(tolower('A') == 'a');
-assert(toupper('a') == 'A');
+assert(std::isalnum('a'));
+assert(std::isalpha('a'));
+assert(std::isblank(' '));
+assert(!std::iscntrl(' '));
+assert(!std::isdigit('a'));
+assert(std::isgraph('a'));
+assert(std::islower('a'));
+assert(std::isprint('a'));
+assert(!std::ispunct('a'));
+assert(!std::isspace('a'));
+assert(!std::isupper('a'));
+assert(std::isxdigit('a'));
+assert(std::tolower('A') == 

Re: [libcxx] r249475 - Remove unnecessary inline functions capturing the contents of C library macros.

[Jonathan Roelofs via cfe-commits]

On 10/6/15 4:03 PM, Richard Smith via cfe-commits wrote:

Author: rsmith
Date: Tue Oct  6 17:03:22 2015
New Revision: 249475

URL: http://llvm.org/viewvc/llvm-project?rev=249475=rev
Log:
Remove unnecessary inline functions capturing the contents of C library macros.

The C standard requires that these be provided as functions even if they're
also provided as macros, and a strict reading of the C++ standard library rules
suggests that (for instance) &::isdigit == &::std::isdigit, so these wrappers
are technically non-conforming.


Mind adding testcases to reinforce those identities, quoting the 
relevant bit(s) of the standard?



Jon





--
Jon Roelofs
jonat...@codesourcery.com
CodeSourcery / Mentor Embedded
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

[Matthias Gehre via cfe-commits]

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

Remove trailing [cppcoreguidelines-pro-type-static-cast-downcast] on tests


http://reviews.llvm.org/D13368

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/ProTypeStaticCastDowncastCheck.cpp
  clang-tidy/cppcoreguidelines/ProTypeStaticCastDowncastCheck.h
  docs/clang-tidy/checks/cppcoreguidelines-pro-type-static-cast-downcast.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-pro-type-static-cast-downcast.cpp

Index: test/clang-tidy/cppcoreguidelines-pro-type-static-cast-downcast.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-pro-type-static-cast-downcast.cpp
@@ -0,0 +1,99 @@
+// RUN: %python %S/check_clang_tidy.py %s cppcoreguidelines-pro-type-static-cast-downcast %t
+
+class Base {
+};
+
+class Derived : public Base {
+};
+
+class Base2 {
+};
+
+class MultiDerived : public Base, public Base2 {
+};
+
+class PolymorphicBase {
+public:
+  virtual ~PolymorphicBase();
+};
+
+class PolymorphicDerived : public PolymorphicBase {
+};
+
+class PolymorphicMultiDerived : public Base, public PolymorphicBase {
+};
+
+void pointers() {
+
+  auto P0 = static_cast(new Base());
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use static_cast to downcast from a base to a derived class [cppcoreguidelines-pro-type-static-cast-downcast]
+
+  auto P1 = static_cast(new Derived()); // OK, upcast to a public base
+  auto P2 = static_cast(new MultiDerived()); // OK, upcast to a public base
+  auto P3 = static_cast(new MultiDerived()); // OK, upcast to a public base
+}
+
+void pointers_polymorphic() {
+
+  auto PP0 = static_cast(new PolymorphicBase());
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: do not use static_cast to downcast from a base to a derived class; use dynamic_cast instead [cppcoreguidelines-pro-type-static-cast-downcast]
+  // CHECK-FIXES: auto PP0 = dynamic_cast(new PolymorphicBase());
+
+  auto B1 = static_cast(new PolymorphicDerived()); // OK, upcast to a public base
+  auto B2 = static_cast(new PolymorphicMultiDerived()); // OK, upcast to a public base
+  auto B3 = static_cast(new PolymorphicMultiDerived()); // OK, upcast to a public base
+}
+
+void arrays() {
+  Base ArrayOfBase[10];
+  auto A0 = static_cast(ArrayOfBase);
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use static_cast to downcast from a base to a derived class [cppcoreguidelines-pro-type-static-cast-downcast]
+}
+
+void arrays_polymorphic() {
+  PolymorphicBase ArrayOfPolymorphicBase[10];
+  auto AP0 = static_cast(ArrayOfPolymorphicBase);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: do not use static_cast to downcast from a base to a derived class; use dynamic_cast instead
+  // CHECK-FIXES: auto AP0 = dynamic_cast(ArrayOfPolymorphicBase);
+}
+
+void references() {
+  Base B0;
+  auto R0 = static_cast(B0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use static_cast to downcast from a base to a derived class [cppcoreguidelines-pro-type-static-cast-downcast]
+  Base& RefToBase = B0;
+  auto R1 = static_cast(RefToBase);
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use static_cast to downcast from a base to a derived class [cppcoreguidelines-pro-type-static-cast-downcast]
+
+  Derived RD1;
+  auto R2 = static_cast(RD1); // OK, upcast to a public base
+}
+
+void references_polymorphic() {
+  PolymorphicBase B0;
+  auto RP0 = static_cast(B0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: do not use static_cast to downcast from a base to a derived class; use dynamic_cast instead
+  // CHECK-FIXES: auto RP0 = dynamic_cast(B0);
+
+
+  PolymorphicBase& RefToPolymorphicBase = B0;
+  auto RP1 = static_cast(RefToPolymorphicBase);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: do not use static_cast to downcast from a base to a derived class; use dynamic_cast instead [cppcoreguidelines-pro-type-static-cast-downcast]
+  // CHECK-FIXES: auto RP1 = dynamic_cast(RefToPolymorphicBase);
+
+  PolymorphicDerived d1;
+  auto RP2 = static_cast(d1); // OK, upcast to a public base
+}
+
+template
+void templ() {
+  auto B0 = static_cast(new D());
+}
+
+void templ_bad_call() {
+  templ(); //FIXME: this should trigger a warning
+}
+
+void templ_good_call() {
+  templ(); // OK, upcast to a public base
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -4,6 +4,7 @@
 .. toctree::
cert-variadic-function-def
   

Re: [PATCH] D13398: [clang-tidy] add check cppcoreguidelines-pro-type-const-cast

[Matthias Gehre via cfe-commits]

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

Rebased


http://reviews.llvm.org/D13398

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.cpp
  clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.h
  docs/clang-tidy/checks/cppcoreguidelines-pro-type-const-cast.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-pro-type-const-cast.cpp

Index: test/clang-tidy/cppcoreguidelines-pro-type-const-cast.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-pro-type-const-cast.cpp
@@ -0,0 +1,6 @@
+// RUN: %python %S/check_clang_tidy.py %s cppcoreguidelines-pro-type-const-cast %t
+
+const int* i;
+int* j;
+void f() { j = const_cast(i); }
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast]
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -3,6 +3,7 @@
 
 .. toctree::
cert-variadic-function-def
+   cppcoreguidelines-pro-type-const-cast
cppcoreguidelines-pro-type-reinterpret-cast
google-build-explicit-make-pair
google-build-namespaces
Index: docs/clang-tidy/checks/cppcoreguidelines-pro-type-const-cast.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/cppcoreguidelines-pro-type-const-cast.rst
@@ -0,0 +1,9 @@
+cppcoreguidelines-pro-type-const-cast
+=
+
+This check flags all uses of const_cast in C++ code.
+
+Modifying a variable that was declared const is undefined behavior, even with const_cast.
+
+This rule is part of the "Type safety" profile of the C++ Core Guidelines, see
+https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#-type3-dont-use-const_cast-to-cast-away-const-ie-at-all.
Index: clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.h
===
--- /dev/null
+++ clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.h
@@ -0,0 +1,34 @@
+//===--- ProTypeConstCastCheck.h - clang-tidy*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_TYPE_CONST_CAST_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_TYPE_CONST_CAST_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+
+/// This check flags all instances of const_cast
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-pro-type-const-cast.html
+class ProTypeConstCastCheck : public ClangTidyCheck {
+public:
+  ProTypeConstCastCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_TYPE_CONST_CAST_H
+
Index: clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.cpp
===
--- /dev/null
+++ clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.cpp
@@ -0,0 +1,33 @@
+//===--- ProTypeConstCastCheck.cpp - clang-tidy===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "ProTypeConstCastCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+
+void ProTypeConstCastCheck::registerMatchers(MatchFinder *Finder) {
+  if(!getLangOpts().CPlusPlus)
+return;
+
+  Finder->addMatcher(cxxConstCastExpr().bind("cast"), this);
+}
+
+void ProTypeConstCastCheck::check(const MatchFinder::MatchResult ) {
+  const auto *MatchedCast = Result.Nodes.getNodeAs("cast");
+  diag(MatchedCast->getOperatorLoc(), "do not use const_cast");
+}
+
+} // namespace tidy
+} // namespace clang
+
Index: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
===
--- clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -10,6 +10,7 @@
 

Re: [PATCH] D13325: Fix crash in codegen on casting to `bool &`.

[John McCall via cfe-commits]

rjmccall added a comment.

LGTM.


http://reviews.llvm.org/D13325



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

Re: [PATCH] D12747: Implement [depr.c.headers]

[Richard Smith via cfe-commits]

Likewise for , , 

On Tue, Oct 6, 2015 at 3:20 PM, Richard Smith  wrote:

> Split  header out of 
>
> On Tue, Oct 6, 2015 at 3:07 PM, Richard Smith 
> wrote:
>
>> Next: factoring the definition of std::nullptr_t out into a separate
>> file, so that  and  can both use it, without 
>> including  and without  providing a ::nullptr_t like
>>  does.
>>
>> On Tue, Oct 6, 2015 at 3:02 PM, Eric Fiselier  wrote:
>>
>>> LGTM.
>>>
>>> On Tue, Oct 6, 2015 at 3:58 PM, Richard Smith 
>>> wrote:
>>> > On Mon, Oct 5, 2015 at 7:10 PM, Eric Fiselier  wrote:
>>> >>
>>> >> EricWF added a comment.
>>> >>
>>> >> I think thing change will help us close a number out outstanding
>>> bugs. I
>>> >> don't have any fundamental objections to this approach.  However the
>>> size of
>>> >> this patch scares me.  I understand the changes are mostly mechanical
>>> but
>>> >> their size can hide things. For example has anybody noticed that your
>>> >> internal changes to `` are in this patch? It would be nice to
>>> break
>>> >> this down into more digestible pieces that could be quickly spot
>>> checked.
>>> >
>>> >
>>> > OK. First such patch is attached. It just removes the macro-capturing
>>> > wrapper functions.
>>>
>>
>>
>
diff --git include/cerrno include/cerrno
index 9804e4e..bab13b8 100644
--- include/cerrno
+++ include/cerrno
@@ -30,364 +30,4 @@ Macros:
 #pragma GCC system_header
 #endif
 
-#if !defined(EOWNERDEAD) || !defined(ENOTRECOVERABLE)
-
-#ifdef ELAST
-
-const int __elast1 = ELAST+1;
-const int __elast2 = ELAST+2;
-
-#else
-
-const int __elast1 = 104;
-const int __elast2 = 105;
-
-#endif
-
-#ifdef ENOTRECOVERABLE
-
-#define EOWNERDEAD __elast1
-
-#ifdef ELAST
-#undef ELAST
-#define ELAST EOWNERDEAD
-#endif
-
-#elif defined(EOWNERDEAD)
-
-#define ENOTRECOVERABLE __elast1
-#ifdef ELAST
-#undef ELAST
-#define ELAST ENOTRECOVERABLE
-#endif
-
-#else  // defined(EOWNERDEAD)
-
-#define EOWNERDEAD __elast1
-#define ENOTRECOVERABLE __elast2
-#ifdef ELAST
-#undef ELAST
-#define ELAST ENOTRECOVERABLE
-#endif
-
-#endif  // defined(EOWNERDEAD)
-
-#endif  // !defined(EOWNERDEAD) || !defined(ENOTRECOVERABLE)
-
-//  supply errno values likely to be missing, particularly on Windows
-
-#ifndef EAFNOSUPPORT
-#define EAFNOSUPPORT 9901
-#endif
-
-#ifndef EADDRINUSE
-#define EADDRINUSE 9902
-#endif
-
-#ifndef EADDRNOTAVAIL
-#define EADDRNOTAVAIL 9903
-#endif
-
-#ifndef EISCONN
-#define EISCONN 9904
-#endif
-
-#ifndef EBADMSG
-#define EBADMSG 9905
-#endif
-
-#ifndef ECONNABORTED
-#define ECONNABORTED 9906
-#endif
-
-#ifndef EALREADY
-#define EALREADY 9907
-#endif
-
-#ifndef ECONNREFUSED
-#define ECONNREFUSED 9908
-#endif
-
-#ifndef ECONNRESET
-#define ECONNRESET 9909
-#endif
-
-#ifndef EDESTADDRREQ
-#define EDESTADDRREQ 9910
-#endif
-
-#ifndef EHOSTUNREACH
-#define EHOSTUNREACH 9911
-#endif
-
-#ifndef EIDRM
-#define EIDRM 9912
-#endif
-
-#ifndef EMSGSIZE
-#define EMSGSIZE 9913
-#endif
-
-#ifndef ENETDOWN
-#define ENETDOWN 9914
-#endif
-
-#ifndef ENETRESET
-#define ENETRESET 9915
-#endif
-
-#ifndef ENETUNREACH
-#define ENETUNREACH 9916
-#endif
-
-#ifndef ENOBUFS
-#define ENOBUFS 9917
-#endif
-
-#ifndef ENOLINK
-#define ENOLINK 9918
-#endif
-
-#ifndef ENODATA
-#define ENODATA 9919
-#endif
-
-#ifndef ENOMSG
-#define ENOMSG 9920
-#endif
-
-#ifndef ENOPROTOOPT
-#define ENOPROTOOPT 9921
-#endif
-
-#ifndef ENOSR
-#define ENOSR 9922
-#endif
-
-#ifndef ENOTSOCK
-#define ENOTSOCK 9923
-#endif
-
-#ifndef ENOSTR
-#define ENOSTR 9924
-#endif
-
-#ifndef ENOTCONN
-#define ENOTCONN 9925
-#endif
-
-#ifndef ENOTSUP
-#define ENOTSUP 9926
-#endif
-
-#ifndef ECANCELED
-#define ECANCELED 9927
-#endif
-
-#ifndef EINPROGRESS
-#define EINPROGRESS 9928
-#endif
-
-#ifndef EOPNOTSUPP
-#define EOPNOTSUPP 9929
-#endif
-
-#ifndef EWOULDBLOCK
-#define EWOULDBLOCK 9930
-#endif
-
-#ifndef EOWNERDEAD
-#define EOWNERDEAD  9931
-#endif
-
-#ifndef EPROTO
-#define EPROTO 9932
-#endif
-
-#ifndef EPROTONOSUPPORT
-#define EPROTONOSUPPORT 9933
-#endif
-
-#ifndef ENOTRECOVERABLE
-#define ENOTRECOVERABLE 9934
-#endif
-
-#ifndef ETIME
-#define ETIME 9935
-#endif
-
-#ifndef ETXTBSY
-#define ETXTBSY 9936
-#endif
-
-#ifndef ETIMEDOUT
-#define ETIMEDOUT 9938
-#endif
-
-#ifndef ELOOP
-#define ELOOP 9939
-#endif
-
-#ifndef EOVERFLOW
-#define EOVERFLOW 9940
-#endif
-
-#ifndef EPROTOTYPE
-#define EPROTOTYPE 9941
-#endif
-
-#ifndef ENOSYS
-#define ENOSYS 9942
-#endif
-
-#ifndef EINVAL
-#define EINVAL 9943
-#endif
-
-#ifndef ERANGE
-#define ERANGE 9944
-#endif
-
-#ifndef EILSEQ
-#define EILSEQ 9945
-#endif
-
-//  Windows Mobile doesn't appear to define these:
-
-#ifndef E2BIG
-#define E2BIG 9946
-#endif
-
-#ifndef EDOM
-#define EDOM 9947
-#endif
-
-#ifndef EFAULT
-#define EFAULT 9948
-#endif
-
-#ifndef EBADF
-#define EBADF 9949
-#endif
-
-#ifndef EPIPE
-#define EPIPE 9950
-#endif
-
-#ifndef EXDEV
-#define EXDEV 9951
-#endif
-
-#ifndef EBUSY
-#define EBUSY 9952
-#endif
-
-#ifndef ENOTEMPTY
-#define 

Re: [PATCH] D12747: Implement [depr.c.headers]

[Richard Smith via cfe-commits]

On Mon, Oct 5, 2015 at 7:10 PM, Eric Fiselier  wrote:

> EricWF added a comment.
>
> I think thing change will help us close a number out outstanding bugs. I
> don't have any fundamental objections to this approach.  However the size
> of this patch scares me.  I understand the changes are mostly mechanical
> but their size can hide things. For example has anybody noticed that your
> internal changes to `` are in this patch? It would be nice to break
> this down into more digestible pieces that could be quickly spot checked.


OK. First such patch is attached. It just removes the macro-capturing
wrapper functions.
Index: cctype
===
--- cctype  (revision 249368)
+++ cctype  (working copy)
@@ -48,117 +48,34 @@
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-#ifdef isalnum
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_isalnum(int __c) {return 
isalnum(__c);}
 #undef isalnum
-inline _LIBCPP_INLINE_VISIBILITY int isalnum(int __c) {return 
__libcpp_isalnum(__c);}
-#else  // isalnum
 using ::isalnum;
-#endif  // isalnum
-
-#ifdef isalpha
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_isalpha(int __c) {return 
isalpha(__c);}
 #undef isalpha
-inline _LIBCPP_INLINE_VISIBILITY int isalpha(int __c) {return 
__libcpp_isalpha(__c);}
-#else  // isalpha
 using ::isalpha;
-#endif  // isalpha
-
-#ifdef isblank
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_isblank(int __c) {return 
isblank(__c);}
 #undef isblank
-inline _LIBCPP_INLINE_VISIBILITY int isblank(int __c) {return 
__libcpp_isblank(__c);}
-#else  // isblank
 using ::isblank;
-#endif  // isblank
-
-#ifdef iscntrl
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_iscntrl(int __c) {return 
iscntrl(__c);}
 #undef iscntrl
-inline _LIBCPP_INLINE_VISIBILITY int iscntrl(int __c) {return 
__libcpp_iscntrl(__c);}
-#else  // iscntrl
 using ::iscntrl;
-#endif  // iscntrl
-
-#ifdef isdigit
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_isdigit(int __c) {return 
isdigit(__c);}
 #undef isdigit
-inline _LIBCPP_INLINE_VISIBILITY int isdigit(int __c) {return 
__libcpp_isdigit(__c);}
-#else  // isdigit
 using ::isdigit;
-#endif  // isdigit
-
-#ifdef isgraph
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_isgraph(int __c) {return 
isgraph(__c);}
 #undef isgraph
-inline _LIBCPP_INLINE_VISIBILITY int isgraph(int __c) {return 
__libcpp_isgraph(__c);}
-#else  // isgraph
 using ::isgraph;
-#endif  // isgraph
-
-#ifdef islower
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_islower(int __c) {return 
islower(__c);}
 #undef islower
-inline _LIBCPP_INLINE_VISIBILITY int islower(int __c) {return 
__libcpp_islower(__c);}
-#else  // islower
 using ::islower;
-#endif  // islower
-
-#ifdef isprint
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_isprint(int __c) {return 
isprint(__c);}
 #undef isprint
-inline _LIBCPP_INLINE_VISIBILITY int isprint(int __c) {return 
__libcpp_isprint(__c);}
-#else  // isprint
 using ::isprint;
-#endif  // isprint
-
-#ifdef ispunct
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_ispunct(int __c) {return 
ispunct(__c);}
 #undef ispunct
-inline _LIBCPP_INLINE_VISIBILITY int ispunct(int __c) {return 
__libcpp_ispunct(__c);}
-#else  // ispunct
 using ::ispunct;
-#endif  // ispunct
-
-#ifdef isspace
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_isspace(int __c) {return 
isspace(__c);}
 #undef isspace
-inline _LIBCPP_INLINE_VISIBILITY int isspace(int __c) {return 
__libcpp_isspace(__c);}
-#else  // isspace
 using ::isspace;
-#endif  // isspace
-
-#ifdef isupper
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_isupper(int __c) {return 
isupper(__c);}
 #undef isupper
-inline _LIBCPP_INLINE_VISIBILITY int isupper(int __c) {return 
__libcpp_isupper(__c);}
-#else  // isupper
 using ::isupper;
-#endif  // isupper
-
-#ifdef isxdigit
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_isxdigit(int __c) {return 
isxdigit(__c);}
 #undef isxdigit
-inline _LIBCPP_INLINE_VISIBILITY int isxdigit(int __c) {return 
__libcpp_isxdigit(__c);}
-#else  // isxdigit
 using ::isxdigit;
-#endif  // isxdigit
-
-#ifdef tolower
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_tolower(int __c) {return 
tolower(__c);}
 #undef tolower
-inline _LIBCPP_INLINE_VISIBILITY int tolower(int __c) {return 
__libcpp_tolower(__c);}
-#else  // tolower
 using ::tolower;
-#endif  // tolower
-
-#ifdef toupper
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_toupper(int __c) {return 
toupper(__c);}
 #undef toupper
-inline _LIBCPP_INLINE_VISIBILITY int toupper(int __c) {return 
__libcpp_toupper(__c);}
-#else  // toupper
 using ::toupper;
-#endif  // toupper
 
 _LIBCPP_END_NAMESPACE_STD
 
Index: cstdio
===
--- cstdio  (revision 249368)
+++ cstdio  (working copy)
@@ -108,36 +108,11 @@
 #include "support/win32/support.h"
 #endif
 
-#ifdef getc
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_getc(FILE* __stream) {return 
getc(__stream);}
 #undef getc
-inline _LIBCPP_INLINE_VISIBILITY int getc(FILE* 

Re: [PATCH] D12747: Implement [depr.c.headers]

[Eric Fiselier via cfe-commits]

LGTM.

On Tue, Oct 6, 2015 at 3:58 PM, Richard Smith  wrote:
> On Mon, Oct 5, 2015 at 7:10 PM, Eric Fiselier  wrote:
>>
>> EricWF added a comment.
>>
>> I think thing change will help us close a number out outstanding bugs. I
>> don't have any fundamental objections to this approach.  However the size of
>> this patch scares me.  I understand the changes are mostly mechanical but
>> their size can hide things. For example has anybody noticed that your
>> internal changes to `` are in this patch? It would be nice to break
>> this down into more digestible pieces that could be quickly spot checked.
>
>
> OK. First such patch is attached. It just removes the macro-capturing
> wrapper functions.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12747: Implement [depr.c.headers]

[Richard Smith via cfe-commits]

Next: factoring the definition of std::nullptr_t out into a separate file,
so that  and  can both use it, without 
including  and without  providing a ::nullptr_t like
 does.

On Tue, Oct 6, 2015 at 3:02 PM, Eric Fiselier  wrote:

> LGTM.
>
> On Tue, Oct 6, 2015 at 3:58 PM, Richard Smith 
> wrote:
> > On Mon, Oct 5, 2015 at 7:10 PM, Eric Fiselier  wrote:
> >>
> >> EricWF added a comment.
> >>
> >> I think thing change will help us close a number out outstanding bugs. I
> >> don't have any fundamental objections to this approach.  However the
> size of
> >> this patch scares me.  I understand the changes are mostly mechanical
> but
> >> their size can hide things. For example has anybody noticed that your
> >> internal changes to `` are in this patch? It would be nice to
> break
> >> this down into more digestible pieces that could be quickly spot
> checked.
> >
> >
> > OK. First such patch is attached. It just removes the macro-capturing
> > wrapper functions.
>
Index: __nullptr
===
--- __nullptr   (revision 0)
+++ __nullptr   (working copy)
@@ -0,0 +1,66 @@
+// -*- C++ -*-
+//===--- __nullptr 
===//
+//
+// 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_NULLPTR
+#define _LIBCPP_NULLPTR
+
+#include <__config>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#pragma GCC system_header
+#endif
+
+#ifdef _LIBCPP_HAS_NO_NULLPTR
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+struct _LIBCPP_TYPE_VIS_ONLY nullptr_t
+{
+void* __lx;
+
+struct __nat {int __for_bool_;};
+
+_LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR nullptr_t() : __lx(0) {}
+_LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR nullptr_t(int __nat::*) : __lx(0) 
{}
+
+_LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR operator int __nat::*() const 
{return 0;}
+
+template 
+_LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR
+operator _Tp* () const {return 0;}
+
+template 
+_LIBCPP_ALWAYS_INLINE
+operator _Tp _Up::* () const {return 0;}
+
+friend _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR bool operator==(nullptr_t, 
nullptr_t) {return true;}
+friend _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR bool operator!=(nullptr_t, 
nullptr_t) {return false;}
+friend _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR bool operator<(nullptr_t, 
nullptr_t) {return false;}
+friend _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR bool operator<=(nullptr_t, 
nullptr_t) {return true;}
+friend _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR bool operator>(nullptr_t, 
nullptr_t) {return false;}
+friend _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR bool operator>=(nullptr_t, 
nullptr_t) {return true;}
+};
+
+inline _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR nullptr_t __get_nullptr_t() 
{return nullptr_t(0);}
+
+#define nullptr _VSTD::__get_nullptr_t()
+
+_LIBCPP_END_NAMESPACE_STD
+
+#else  // _LIBCPP_HAS_NO_NULLPTR
+
+namespace std
+{
+typedef decltype(nullptr) nullptr_t;
+}
+
+#endif  // _LIBCPP_HAS_NO_NULLPTR
+
+#endif  // _LIBCPP_NULLPTR
Index: cstddef
===
--- cstddef (revision 249368)
+++ cstddef (working copy)
@@ -34,6 +34,7 @@
 */
 
 #include <__config>
+#include <__nullptr>
 
 #include 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D10305: [Clang Static Analyzer] Bug identification

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

xazax.hun marked 2 inline comments as done.
xazax.hun added a comment.

In http://reviews.llvm.org/D10305#258671, @zaks.anna wrote:

> > Generating mangled names requires ASTContext which is not available during 
> > the error reporting. BugReporter does have the ASTContext, so it would not 
>
> >  be a big change to add it to the DiagnosticConsumers though. And I think 
> > the mangled name might contain too much redundant information. The only 
>
> >  relevant information here is the fully qualified name and the parts of the 
> > signature that can be ocerloaded on e.g.: constness. Using this method 
> > might also 
>
> >  be easier to debug, since the IssueString is more readable.
>
>
> I think it'd be fine to pass ASTContext to the DiagnosticConsumers. It would 
> be useful to find out exactly what extra information the mangled names have 
> that we do not want to see in the issue_hash.


I looked into the name mangling and found the following downsides:

- The name mangling is different when clang is running in msvc compatible mode. 
If we want to consistent hashes on all platforms, we should not rely on mangled 
names.
- The name mangling contains the calling convention which is redundant 
information.
- Some attributes have effect on name mangling.
- Linkage is included in mangled name.

In general using mangled name might cause unexpected behaviour for users. E.g. 
a user might  not expect that making a function extern "C" changes the hashes 
in the function.

> One issue that I see with the current approach is that we do not 
> differentiate methods from different classes/namespaces. Is this by design?


The implementation do differentiate. It uses qualified names whenever a name is 
queried, which contains all of the enclosing namespaces and classes.

> 

> 

> > I definitely agree. It would be great to have a unique identifier for 
> > checkers that is independent from the name/package. It is not a trivial 
> > problem however,

> 

> >  since we probably want to support plugins too. I can think of a similar 
> > solution like git commit ids, but I think this problem might be out of the 
> > scope of this

> 

> >  patch. But I am happy to start a discussion on the mailing list and create 
> > a new patch to solve this.

> 

> 

> Sounds good to me. I agree that this is out of scope for this patch.





http://reviews.llvm.org/D10305



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

Re: [PATCH] D13100: [mips] Separated mips specific -Wa options, so that they are not checked on other platforms.

[Daniel Sanders via cfe-commits]

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

LGTM


http://reviews.llvm.org/D13100



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

Re: [PATCH] D13326: [PGO]: Eliminate __llvm_profile_register calls for Linux (clang changes)

[Xinliang David Li via cfe-commits]

Clang FE refactoring change is not needed anymore.

In commit 0d32e7d952bc80830183e0c2c6ec5027ca6b1450, Vasileios
Kalintiris did the same thing for multi-lib support.

David

On Tue, Oct 6, 2015 at 12:13 AM, Justin Bogner  wrote:
> David Li  writes:
>> davidxl updated this revision to Diff 36316.
>> davidxl added a comment.
>>
>> I have modified the implementation to not use linker script, so this
>> clang patch becomes strictly refactoring with NFC. I think it is still
>> a good thing to have this in so that similar tunings like this can be
>> done in the future.
>
> Sure. I have a couple of nitpicks but this basically LGTM.
>
>>
>> http://reviews.llvm.org/D13326
>>
>> Files:
>>   include/clang/Driver/ToolChain.h
>>   lib/Driver/SanitizerArgs.cpp
>>   lib/Driver/ToolChain.cpp
>>   lib/Driver/ToolChains.cpp
>>   lib/Driver/ToolChains.h
>>   lib/Driver/Tools.cpp
>>
>> Index: lib/Driver/Tools.cpp
>> ===
>> --- lib/Driver/Tools.cpp
>> +++ lib/Driver/Tools.cpp
>> @@ -2402,83 +2402,12 @@
>>}
>>  }
>>
>> -// Until ARM libraries are build separately, we have them all in one library
>> -static StringRef getArchNameForCompilerRTLib(const ToolChain ,
>> - const ArgList ) {
>> -  const llvm::Triple  = TC.getTriple();
>> -  bool IsWindows = Triple.isOSWindows();
>> -
>> -  if (Triple.isWindowsMSVCEnvironment() && TC.getArch() == 
>> llvm::Triple::x86)
>> -return "i386";
>> -
>> -  if (TC.getArch() == llvm::Triple::arm || TC.getArch() == 
>> llvm::Triple::armeb)
>> -return (arm::getARMFloatABI(TC, Args) == arm::FloatABI::Hard && 
>> !IsWindows)
>> -   ? "armhf"
>> -   : "arm";
>> -
>> -  return TC.getArchName();
>> -}
>> -
>> -static SmallString<128> getCompilerRTLibDir(const ToolChain ) {
>> -  // The runtimes are located in the OS-specific resource directory.
>> -  SmallString<128> Res(TC.getDriver().ResourceDir);
>> -  const llvm::Triple  = TC.getTriple();
>> -  // TC.getOS() yield "freebsd10.0" whereas "freebsd" is expected.
>> -  StringRef OSLibName =
>> -  (Triple.getOS() == llvm::Triple::FreeBSD) ? "freebsd" : TC.getOS();
>> -  llvm::sys::path::append(Res, "lib", OSLibName);
>> -  return Res;
>> -}
>> -
>> -SmallString<128> tools::getCompilerRT(const ToolChain , const ArgList 
>> ,
>> -  StringRef Component, bool Shared) {
>> -  const char *Env = TC.getTriple().getEnvironment() == llvm::Triple::Android
>> -? "-android"
>> -: "";
>> -
>> -  bool IsOSWindows = TC.getTriple().isOSWindows();
>> -  bool IsITANMSVCWindows = TC.getTriple().isWindowsMSVCEnvironment() ||
>> -   TC.getTriple().isWindowsItaniumEnvironment();
>> -  StringRef Arch = getArchNameForCompilerRTLib(TC, Args);
>> -  const char *Prefix = IsITANMSVCWindows ? "" : "lib";
>> -  const char *Suffix =
>> -  Shared ? (IsOSWindows ? ".dll" : ".so") : (IsITANMSVCWindows ? ".lib" 
>> : ".a");
>> -
>> -  SmallString<128> Path = getCompilerRTLibDir(TC);
>> -  llvm::sys::path::append(Path, Prefix + Twine("clang_rt.") + Component + 
>> "-" +
>> -Arch + Env + Suffix);
>> -
>> -  return Path;
>> -}
>> -
>> -static const char *getCompilerRTArgString(const ToolChain ,
>> -  const llvm::opt::ArgList ,
>> -  StringRef Component,
>> -  bool Shared = false) {
>> -  return Args.MakeArgString(getCompilerRT(TC, Args, Component, Shared));
>> -}
>> -
>>  // This adds the static libclang_rt.builtins-arch.a directly to the command 
>> line
>>  // FIXME: Make sure we can also emit shared objects if they're requested
>>  // and available, check for possible errors, etc.
>>  static void addClangRT(const ToolChain , const ArgList ,
>> ArgStringList ) {
>> -  CmdArgs.push_back(getCompilerRTArgString(TC, Args, "builtins"));
>> -}
>> -
>> -static void addProfileRT(const ToolChain , const ArgList ,
>> - ArgStringList ) {
>> -  if (!(Args.hasFlag(options::OPT_fprofile_arcs, 
>> options::OPT_fno_profile_arcs,
>> - false) ||
>> -Args.hasArg(options::OPT_fprofile_generate) ||
>> -Args.hasArg(options::OPT_fprofile_generate_EQ) ||
>> -Args.hasArg(options::OPT_fprofile_instr_generate) ||
>> -Args.hasArg(options::OPT_fprofile_instr_generate_EQ) ||
>> -Args.hasArg(options::OPT_fcreate_profile) ||
>> -Args.hasArg(options::OPT_coverage)))
>> -return;
>> -
>> -  CmdArgs.push_back(getCompilerRTArgString(TC, Args, "profile"));
>> +  CmdArgs.push_back(TC.getCompilerRTArgString(Args, "builtins"));
>>  }
>>
>>  namespace {
>> @@ -2559,7 +2488,7 @@
>>// whole-archive.
>>if (!IsShared)
>>  

Re: [PATCH] D12501: [clang-format] Obj-C dictionary literals: Fixed typecast getting put on a separate line from the key

[Kent Sutherland via cfe-commits]

ksuther added a comment.

Do I need to do anything else about this and http://reviews.llvm.org/D12489, or 
do they eventually get committed by someone else? Thanks!


http://reviews.llvm.org/D12501



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

Re: [PATCH] D13407: [libcxx] Capture configuration information when installing the libc++ headers

[Eric Fiselier via cfe-commits]

EricWF added a comment.

Cool. I've reached out to some platform maintainers to make sure they don't see 
any problems.  I'll commit this once I hear back.


http://reviews.llvm.org/D13407



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

Re: [PATCH] D12747: Implement [depr.c.headers]

[Richard Smith via cfe-commits]

On Tue, Oct 6, 2015 at 4:16 PM, Sean Silva  wrote:

> On Tue, Oct 6, 2015 at 4:13 PM, Richard Smith 
> wrote:
>
>> On Tue, Oct 6, 2015 at 4:11 PM, Sean Silva  wrote:
>>
>>> +extern "C++" {
>>> +#include <__nullptr>
>>> +using std::nullptr_t;
>>> +}
>>>
>>> Does this even compile with modules?
>>>
>>
>> Yes. You're allowed to import a C++ module within an 'extern "C++"' block.
>>
>
> Why do we even need `extern "C++"` if we are already under `#ifdef
> __cplusplus`?
>

People like to include C++'s  headers inside 'extern "C"' blocks. We
shouldn't break that if we don't have to.


> -- Sean Silva
>
>
>>
>>
>>> On Tue, Oct 6, 2015 at 3:57 PM, Richard Smith via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
 . This one is tricky:

 1) There's an (undocumented) interface between the C standard library
 and this header, where the macros __need_ptrdiff_t, __need_size_t,
 __need_wchar_t, __need_NULL, __need_wint_t request just a piece of this
 header rather than the whole thing. If we see any of those, just go
 straight to the underlying header.

 2) We probably don't want  to include  (for
 consistency with other headers), but  must provide a ::nullptr_t
 (which we don't want  to provide). So neither header includes the
 other. Instead, both include <__nullptr> for std::nullptr_t, and we
 duplicate the definition of max_align_t between them, in the case where the
 compiler's  doesn't provide it.

 If you prefer, I could make  include  to avoid the
 duplication of the max_align_t logic.

 This patch depends on patch 02.

 On Tue, Oct 6, 2015 at 3:42 PM, Richard Smith 
 wrote:

> , an easy one. We guarantee a setjmp macro exists even if
> this header is somehow included from C (the C standard allows that, so 
> it's
> not worth checking for __cplusplus).
>
> On Tue, Oct 6, 2015 at 3:36 PM, Richard Smith 
> wrote:
>
>> Split  out of . This is a big change, but the same
>> pattern as the prior ones.
>>
>> On Tue, Oct 6, 2015 at 3:28 PM, Richard Smith 
>> wrote:
>>
>>> Likewise for , , 
>>>
>>> On Tue, Oct 6, 2015 at 3:20 PM, Richard Smith >> > wrote:
>>>
 Split  header out of 

 On Tue, Oct 6, 2015 at 3:07 PM, Richard Smith <
 rich...@metafoo.co.uk> wrote:

> Next: factoring the definition of std::nullptr_t out into a
> separate file, so that  and  can both use it, 
> without
>  including  and without  providing a
> ::nullptr_t like  does.
>
> On Tue, Oct 6, 2015 at 3:02 PM, Eric Fiselier 
> wrote:
>
>> LGTM.
>>
>> On Tue, Oct 6, 2015 at 3:58 PM, Richard Smith <
>> rich...@metafoo.co.uk> wrote:
>> > On Mon, Oct 5, 2015 at 7:10 PM, Eric Fiselier 
>> wrote:
>> >>
>> >> EricWF added a comment.
>> >>
>> >> I think thing change will help us close a number out
>> outstanding bugs. I
>> >> don't have any fundamental objections to this approach.
>> However the size of
>> >> this patch scares me.  I understand the changes are mostly
>> mechanical but
>> >> their size can hide things. For example has anybody noticed
>> that your
>> >> internal changes to `` are in this patch? It would be
>> nice to break
>> >> this down into more digestible pieces that could be quickly
>> spot checked.
>> >
>> >
>> > OK. First such patch is attached. It just removes the
>> macro-capturing
>> > wrapper functions.
>>
>
>

>>>
>>
>

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


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

Re: [PATCH] D9898: MismatchingNewDeleteDetector uses incorrect field, and finds no initializer

[Richard Smith via cfe-commits]

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

LGTM


http://reviews.llvm.org/D9898



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

Re: [clang-tools-extra] r249235 - Replace double negation of !FileID.isInvalid() with FileID.isValid().

[Alexander Kornienko via cfe-commits]

I'd say that the SourceLocation::isInvalid is more confusing than useful.
Not sure why it was added in the first place.

On Sat, Oct 3, 2015 at 12:46 PM, Yaron Keren via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: yrnkrn
> Date: Sat Oct  3 05:46:20 2015
> New Revision: 249235
>
> URL: http://llvm.org/viewvc/llvm-project?rev=249235=rev
> Log:
> Replace double negation of !FileID.isInvalid() with FileID.isValid().
> +couple more of double-negated !SourceLocation.isInvalid() unfixed in
> r249228.
>
>
> Modified:
>
> clang-tools-extra/trunk/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
> clang-tools-extra/trunk/clang-modernize/Core/IncludeDirectives.cpp
> clang-tools-extra/trunk/clang-modernize/LoopConvert/LoopActions.cpp
> clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.cpp
>
> Modified:
> clang-tools-extra/trunk/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp?rev=249235=249234=249235=diff
>
> ==
> ---
> clang-tools-extra/trunk/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
> (original)
> +++
> clang-tools-extra/trunk/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
> Sat Oct  3 05:46:20 2015
> @@ -118,7 +118,7 @@ getRewrittenData(const std::vectorconst clang::FileEntry *Entry = Files.getFile(FileName);
>assert(Entry && "Expected an existing file");
>FileID ID = SM.translateFile(Entry);
> -  assert(!ID.isInvalid() && "Expected a valid FileID");
> +  assert(ID.isValid() && "Expected a valid FileID");
>const RewriteBuffer *Buffer = Rewrites.getRewriteBufferFor(ID);
>Result = std::string(Buffer->begin(), Buffer->end());
>
>
> Modified:
> clang-tools-extra/trunk/clang-modernize/Core/IncludeDirectives.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-modernize/Core/IncludeDirectives.cpp?rev=249235=249234=249235=diff
>
> ==
> --- clang-tools-extra/trunk/clang-modernize/Core/IncludeDirectives.cpp
> (original)
> +++ clang-tools-extra/trunk/clang-modernize/Core/IncludeDirectives.cpp Sat
> Oct  3 05:46:20 2015
> @@ -354,7 +354,7 @@ bool IncludeDirectives::hasInclude(const
>  Replacement IncludeDirectives::addAngledInclude(const clang::FileEntry
> *File,
>  llvm::StringRef Include) {
>FileID FID = Sources.translateFile(File);
> -  assert(!FID.isInvalid() && "Invalid file entry given!");
> +  assert(FID.isValid() && "Invalid file entry given!");
>
>if (hasInclude(File, Include))
>  return Replacement();
>
> Modified:
> clang-tools-extra/trunk/clang-modernize/LoopConvert/LoopActions.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-modernize/LoopConvert/LoopActions.cpp?rev=249235=249234=249235=diff
>
> ==
> --- clang-tools-extra/trunk/clang-modernize/LoopConvert/LoopActions.cpp
> (original)
> +++ clang-tools-extra/trunk/clang-modernize/LoopConvert/LoopActions.cpp
> Sat Oct  3 05:46:20 2015
> @@ -578,7 +578,7 @@ bool ForLoopIndexUseVisitor::TraverseMem
> Context->getLangOpts());
>  // If something complicated is happening (i.e. the next token isn't an
>  // arrow), give up on making this work.
> -if (!ArrowLoc.isInvalid()) {
> +if (ArrowLoc.isValid()) {
>Usages.push_back(Usage(ResultExpr, /*IsArrow=*/true,
>   SourceRange(Base->getExprLoc(), ArrowLoc)));
>return true;
>
> Modified: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.cpp?rev=249235=249234=249235=diff
>
> ==
> --- clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.cpp
> (original)
> +++ clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.cpp Sat
> Oct  3 05:46:20 2015
> @@ -559,7 +559,7 @@ bool ForLoopIndexUseVisitor::TraverseMem
>  Context->getLangOpts());
>  // If something complicated is happening (i.e. the next token isn't an
>  // arrow), give up on making this work.
> -if (!ArrowLoc.isInvalid()) {
> +if (ArrowLoc.isValid()) {
>addUsage(Usage(ResultExpr, Usage::UK_MemberThroughArrow,
>   SourceRange(Base->getExprLoc(), ArrowLoc)));
>return true;
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list

Re: [PATCH] D12644: Using -isysroot on Apple platform

[Eric Fiselier via cfe-commits]

EricWF added a comment.

You need to use the CMake option `CMAKE_OSX_SYSROOT` to manage the system root 
because `lib/CMakeLists.txt` uses "CMAKE_OSX_SYSROOT" to find the correct 
libc++abi.dylib.
Have you tried that? It is unfortunate that Linux and OS X use different 
options for the same thing though. Maybe we could set `CMAKE_OSX_SYSROOT` 
internally based on `LIBCXX_SYSROOT`?


http://reviews.llvm.org/D12644



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

r249497 - [SEH] Fix x64 __exception_code in __except blocks

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Tue Oct  6 20:07:13 2015
New Revision: 249497

URL: http://llvm.org/viewvc/llvm-project?rev=249497=rev
Log:
[SEH] Fix x64 __exception_code in __except blocks

Use llvm.eh.exceptioncode to get the code out of EAX for x64. For
32-bit, the filter is responsible for storing it to memory for us.

Modified:
cfe/trunk/lib/CodeGen/CGException.cpp
cfe/trunk/test/CodeGen/exceptions-seh-new.c

Modified: cfe/trunk/lib/CodeGen/CGException.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGException.cpp?rev=249497=249496=249497=diff
==
--- cfe/trunk/lib/CodeGen/CGException.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGException.cpp Tue Oct  6 20:07:13 2015
@@ -1898,14 +1898,21 @@ void CodeGenFunction::ExitSEHTryStmt(con
 ExceptBB = createBasicBlock("__except");
 Builder.CreateCatchRet(CPI, ExceptBB);
 EmitBlock(ExceptBB);
-  }
-
-  // On Win64, the exception pointer is the exception code. Copy it to the 
slot.
-  if (CGM.getTarget().getTriple().getArch() != llvm::Triple::x86) {
-llvm::Value *Code =
-Builder.CreatePtrToInt(getExceptionFromSlot(), IntPtrTy);
-Code = Builder.CreateTrunc(Code, Int32Ty);
-Builder.CreateStore(Code, SEHCodeSlotStack.back());
+// On Win64, the exception code is returned in EAX. Copy it into the slot.
+if (CGM.getTarget().getTriple().getArch() != llvm::Triple::x86) {
+  llvm::Function *SEHCodeIntrin =
+  CGM.getIntrinsic(llvm::Intrinsic::eh_exceptioncode);
+  llvm::Value *Code = Builder.CreateCall(SEHCodeIntrin, {CPI});
+  Builder.CreateStore(Code, SEHCodeSlotStack.back());
+}
+  } else {
+// On Win64, the exception pointer is the exception code. Copy it to the 
slot.
+if (CGM.getTarget().getTriple().getArch() != llvm::Triple::x86) {
+  llvm::Value *Code =
+  Builder.CreatePtrToInt(getExceptionFromSlot(), IntPtrTy);
+  Code = Builder.CreateTrunc(Code, Int32Ty);
+  Builder.CreateStore(Code, SEHCodeSlotStack.back());
+}
   }
 
   // Emit the __except body.

Modified: cfe/trunk/test/CodeGen/exceptions-seh-new.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/exceptions-seh-new.c?rev=249497=249496=249497=diff
==
--- cfe/trunk/test/CodeGen/exceptions-seh-new.c (original)
+++ cfe/trunk/test/CodeGen/exceptions-seh-new.c Tue Oct  6 20:07:13 2015
@@ -265,4 +265,25 @@ void finally_capture_twice(int x) {
 // CHECK-NEXT:store i32 [[T0]], i32* [[Z]], align 4
 // CHECK-NEXT:ret void
 
+int exception_code_in_except(void) {
+  __try {
+try_body(0, 0, 0);
+  } __except(1) {
+return _exception_code();
+  }
+}
+
+// CHECK-LABEL: define i32 @exception_code_in_except()
+// CHECK: %[[ret_slot:[^ ]*]] = alloca i32
+// CHECK: %[[code_slot:[^ ]*]] = alloca i32
+// CHECK: invoke void @try_body(i32 0, i32 0, i32* null)
+// CHECK: %[[pad:[^ ]*]] = catchpad
+// CHECK: catchret %[[pad]]
+// X64: %[[code:[^ ]*]] = call i32 @llvm.eh.exceptioncode(token %[[pad]])
+// X64: store i32 %[[code]], i32* %[[code_slot]]
+// CHECK: %[[ret1:[^ ]*]] = load i32, i32* %[[code_slot]]
+// CHECK: store i32 %[[ret1]], i32* %[[ret_slot]]
+// CHECK: %[[ret2:[^ ]*]] = load i32, i32* %[[ret_slot]]
+// CHECK: ret i32 %[[ret2]]
+
 // CHECK: attributes #[[NOINLINE]] = { {{.*noinline.*}} }


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

Re: [PATCH] D13488: [analyzer] Assume escape is possible through system functions taking void*

[Devin Coughlin via cfe-commits]

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

Has a typo but otherwise LGTM.



Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:258
@@ +257,3 @@
+  /// the condition.
+  bool hasNonNullArgumentsWithType(bool (*Condition)(QualType)) const;
+

Did you consider using `std::function` instead of a function 
pointer so that clients could use lambdas that capture to provide the condition?


Comment at: test/Analysis/Inputs/system-header-simulator.h:85
@@ -84,1 +84,3 @@
 
+// Some data strauctures may hold onto the pointer and free it later.
+void fake_insque(void *, void *);

Typo "strauctures"


http://reviews.llvm.org/D13488



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

r249499 - clang/test/CodeGen/exceptions-seh-leave-new.c: Use "opt -instnamer" for branch-sensitive checks.

[NAKAMURA Takumi via cfe-commits]

Author: chapuni
Date: Tue Oct  6 20:29:26 2015
New Revision: 249499

URL: http://llvm.org/viewvc/llvm-project?rev=249499=rev
Log:
clang/test/CodeGen/exceptions-seh-leave-new.c: Use "opt -instnamer" for 
branch-sensitive checks.

Modified:
cfe/trunk/test/CodeGen/exceptions-seh-leave-new.c

Modified: cfe/trunk/test/CodeGen/exceptions-seh-leave-new.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/exceptions-seh-leave-new.c?rev=249499=249498=249499=diff
==
--- cfe/trunk/test/CodeGen/exceptions-seh-leave-new.c (original)
+++ cfe/trunk/test/CodeGen/exceptions-seh-leave-new.c Tue Oct  6 20:29:26 2015
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -triple x86_64-pc-win32 -fms-extensions -fnew-ms-eh 
-emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -triple x86_64-pc-win32 -fms-extensions -fnew-ms-eh 
-emit-llvm -o - | opt -instnamer -S | FileCheck %s
 
 void g(void);
 


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

r249501 - clang-format: Fix false positive in pointer/reference detection.

[Daniel Jasper via cfe-commits]

Author: djasper
Date: Tue Oct  6 20:41:14 2015
New Revision: 249501

URL: http://llvm.org/viewvc/llvm-project?rev=249501=rev
Log:
clang-format: Fix false positive in pointer/reference detection.

Before:
  return options != nullptr &==(*options);

After:
  return options != nullptr && operator==(*options);

Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=249501=249500=249501=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Tue Oct  6 20:41:14 2015
@@ -1158,7 +1158,9 @@ private:
 
 if (NextToken->is(tok::l_square) && NextToken->isNot(TT_LambdaLSquare))
   return TT_PointerOrReference;
-if (NextToken->isOneOf(tok::kw_operator, tok::comma, tok::semi))
+if (NextToken->is(tok::kw_operator) && !IsExpression)
+  return TT_PointerOrReference;
+if (NextToken->isOneOf(tok::comma, tok::semi))
   return TT_PointerOrReference;
 
 if (PrevToken->is(tok::r_paren) && PrevToken->MatchingParen &&

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=249501=249500=249501=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Tue Oct  6 20:41:14 2015
@@ -5568,6 +5568,7 @@ TEST_F(FormatTest, UnderstandsUsesOfStar
   // verifyIndependentOfContext("MACRO(A *a);");
 
   verifyFormat("DatumHandle const *operator->() const { return input_; }");
+  verifyFormat("return options != nullptr && operator==(*options);");
 
   EXPECT_EQ("#define OP(x)\\\n"
 "  ostream <<(ostream , const A ) {  \\\n"


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

r249502 - clang-format: Understand array reference types.

[Daniel Jasper via cfe-commits]

Author: djasper
Date: Tue Oct  6 20:41:22 2015
New Revision: 249502

URL: http://llvm.org/viewvc/llvm-project?rev=249502=rev
Log:
clang-format: Understand array reference types.

Before:
  void f(Type()[10]) {}
  void f(Type (*parameter)[10]) {}

After:
  void f(Type ()[10]) {}
  void f(Type (*parameter)[10]) {}

Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=249502=249501=249502=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Tue Oct  6 20:41:22 2015
@@ -155,7 +155,7 @@ private:
   Left->Type = TT_ObjCMethodExpr;
 }
 
-bool MightBeFunctionType = CurrentToken->is(tok::star);
+bool MightBeFunctionType = CurrentToken->isOneOf(tok::star, tok::amp);
 bool HasMultipleLines = false;
 bool HasMultipleParametersOnALine = false;
 bool MightBeObjCForRangeLoop =

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=249502=249501=249502=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Tue Oct  6 20:41:22 2015
@@ -5423,6 +5423,7 @@ TEST_F(FormatTest, UnderstandsUsesOfStar
   verifyIndependentOfContext("return (int **&)a;");
   verifyIndependentOfContext("f((*PointerToArray)[10]);");
   verifyFormat("void f(Type (*parameter)[10]) {}");
+  verifyFormat("void f(Type ()[10]) {}");
   verifyGoogleFormat("return sizeof(int**);");
   verifyIndependentOfContext("Type **A = static_cast(P);");
   verifyGoogleFormat("Type** A = static_cast(P);");


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

Re: [PATCH] D13099: [Analyzer] Don’t invalidate CXXThis when conservatively evaluating const methods (PR 21606)

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

xazax.hun added inline comments.


Comment at: test/Analysis/const-method-call.cpp:26
@@ +25,3 @@
+
+struct Derived : Base {
+  mutable int mut;

Please add a  test case, where only base have a mutable field.


http://reviews.llvm.org/D13099



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

Re: [PATCH] D13099: [Analyzer] Don’t invalidate CXXThis when conservatively evaluating const methods (PR 21606)

[Sean Eveson via cfe-commits]

seaneveson marked 6 inline comments as done.
seaneveson added a comment.

There is an issue where pointers to the object (this) should cause it to be 
invalidated, but don't since TK_PreserveContents has been set.

For example:

  class B;
  class A {
B b;
const foo();
  };
  class B {
A *ptr_a;
  }
  
  A a;
  a.b.ptr_a = 
  a.foo();

The method foo might modify 'this' via the object b, but 'this' will not be 
invalidated as foo is const.

Again I'm not sure this is worth checking for, based on the assumption that a 
reasonable const method won't modify the relevant object. What do people think?


http://reviews.llvm.org/D13099



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

Re: [PATCH] D13099: [Analyzer] Don’t invalidate CXXThis when conservatively evaluating const methods (PR 21606)

[Sean Eveson via cfe-commits]

seaneveson updated this revision to Diff 36604.
seaneveson added a comment.

Removed mutable field from derived class in inheritance test case.


http://reviews.llvm.org/D13099

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  lib/StaticAnalyzer/Core/CallEvent.cpp
  test/Analysis/PR21606.cpp
  test/Analysis/const-method-call.cpp

Index: test/Analysis/const-method-call.cpp
===
--- test/Analysis/const-method-call.cpp
+++ test/Analysis/const-method-call.cpp
@@ -0,0 +1,89 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(bool);
+
+struct A {
+  int x;
+  void foo() const;
+  void bar();
+};
+
+struct B {
+  mutable int mut;
+  void foo() const;
+};
+
+struct C {
+  int *p;
+  void foo() const;
+};
+
+struct Base {
+  mutable int b_mut;
+  int *p;
+};
+
+struct Derived : Base {
+  void foo() const;
+};
+
+struct Inner {
+  int *p;
+};
+
+struct Outer {
+  int x;
+  Inner in;
+  void foo() const;
+};
+
+void checkThatConstMethodWithoutDefinitionDoesNotInvalidateObject() {
+  A t;
+  t.x = 3;
+  t.foo();
+  clang_analyzer_eval(t.x == 3); // expected-warning{{TRUE}}
+  // Test non-const does invalidate
+  t.bar();
+  clang_analyzer_eval(t.x); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatConstMethodDoesInvalidateMutableFields() {
+  B t;
+  t.mut = 4;
+  t.foo();
+  clang_analyzer_eval(t.mut); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatConstMethodDoesInvalidatePointedAtMemory() {
+  int x = 1;
+  C t;
+  t.p = 
+  t.foo();
+  clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatConstMethodDoesInvalidateInheritedMutableFields() {
+  Derived t;
+  t.b_mut = 4;
+  t.foo();
+  clang_analyzer_eval(t.b_mut); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatConstMethodDoesInvalidateInheritedPointedAtMemory() {
+  int x = 1;
+  Derived t;
+  t.p = 
+  t.foo();
+  clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatConstMethodDoesInvalidateContainedPointedAtMemory() {
+  int x = 1;
+  Outer t;
+  t.x = 2;
+  t.in.p = 
+  t.foo();
+  clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(t.x == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(t.in.p == ); // expected-warning{{TRUE}}
+}
Index: test/Analysis/PR21606.cpp
===
--- test/Analysis/PR21606.cpp
+++ test/Analysis/PR21606.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core
+// PR21606
+
+struct s1 {
+void g(const int *i) const;
+};
+
+struct s2 {
+void f(int *i) {
+m_i = i;
+m_s.g(m_i);
+if (m_i)
+*i = 42; // no-warning
+}
+
+int *m_i;
+s1 m_s;
+};
+
+int main()
+{
+s2().f(0);
+}
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -148,7 +148,7 @@
   SmallVector ValuesToInvalidate;
   RegionAndSymbolInvalidationTraits ETraits;
 
-  getExtraInvalidatedValues(ValuesToInvalidate);
+  getExtraInvalidatedValues(ValuesToInvalidate, );
 
   // Indexes of arguments whose values will be preserved by the call.
   llvm::SmallSet PreserveArgs;
@@ -403,8 +403,20 @@
   return getSVal(CE->getCallee()).getAsFunctionDecl();
 }
 
-void CXXInstanceCall::getExtraInvalidatedValues(ValueList ) const {
-  Values.push_back(getCXXThisVal());
+void CXXInstanceCall::getExtraInvalidatedValues(ValueList ,
+RegionAndSymbolInvalidationTraits *ETraits) const {
+  SVal ThisVal = getCXXThisVal();
+  Values.push_back(ThisVal);
+
+  // Don't invalitate if the method is const and there are no mutable fields
+  if (const CXXMethodDecl *D = cast_or_null(getDecl())) {
+if (D->isConst() && !D->getParent()->hasMutableFields()) {
+  const MemRegion *ThisRegion = ThisVal.getAsRegion();
+  assert(ThisRegion && "ThisValue was not a memory region");
+  ETraits->setTrait(ThisRegion->StripCasts(),
+RegionAndSymbolInvalidationTraits::TK_PreserveContents);
+}
+  }
 }
 
 SVal CXXInstanceCall::getCXXThisVal() const {
@@ -550,7 +562,8 @@
   return D->parameters();
 }
 
-void BlockCall::getExtraInvalidatedValues(ValueList ) const {
+void BlockCall::getExtraInvalidatedValues(ValueList ,
+  RegionAndSymbolInvalidationTraits *ETraits) const {
   // FIXME: This also needs to invalidate captured globals.
   if (const MemRegion *R = getBlockRegion())
 Values.push_back(loc::MemRegionVal(R));
@@ -571,7 +584,8 @@
   return UnknownVal();
 }
 
-void CXXConstructorCall::getExtraInvalidatedValues(ValueList ) const {
+void CXXConstructorCall::getExtraInvalidatedValues(ValueList ,
+   RegionAndSymbolInvalidationTraits *ETraits) const {
   if (Data)
 

Re: [PATCH] D13099: [Analyzer] Don’t invalidate CXXThis when conservatively evaluating const methods (PR 21606)

[Sean Eveson via cfe-commits]

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

http://reviews.llvm.org/D13099



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

Re: [PATCH] D12747: Implement [depr.c.headers]

[Richard Smith via cfe-commits]

On Tue, Oct 6, 2015 at 3:07 PM, Richard Smith  wrote:

> Next: factoring the definition of std::nullptr_t out into a separate file,
> so that  and  can both use it, without 
> including  and without  providing a ::nullptr_t like
>  does.
>

Sorry, missed a couple of the hunks for this patch.


> On Tue, Oct 6, 2015 at 3:02 PM, Eric Fiselier  wrote:
>
>> LGTM.
>>
>> On Tue, Oct 6, 2015 at 3:58 PM, Richard Smith 
>> wrote:
>> > On Mon, Oct 5, 2015 at 7:10 PM, Eric Fiselier  wrote:
>> >>
>> >> EricWF added a comment.
>> >>
>> >> I think thing change will help us close a number out outstanding bugs.
>> I
>> >> don't have any fundamental objections to this approach.  However the
>> size of
>> >> this patch scares me.  I understand the changes are mostly mechanical
>> but
>> >> their size can hide things. For example has anybody noticed that your
>> >> internal changes to `` are in this patch? It would be nice to
>> break
>> >> this down into more digestible pieces that could be quickly spot
>> checked.
>> >
>> >
>> > OK. First such patch is attached. It just removes the macro-capturing
>> > wrapper functions.
>>
>
>
commit 89b8a2875ac1cf084213ad47623cd92c4a087cc5
Author: Richard Smith 
Date:   Tue Oct 6 15:12:30 2015 -0700

Factor out std::nullptr_t into a separate file.

diff --git include/__nullptr include/__nullptr
new file mode 100644
index 000..95415a6
--- /dev/null
+++ include/__nullptr
@@ -0,0 +1,66 @@
+// -*- C++ -*-
+//===--- __nullptr 
===//
+//
+// 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_NULLPTR
+#define _LIBCPP_NULLPTR
+
+#include <__config>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#pragma GCC system_header
+#endif
+
+#ifdef _LIBCPP_HAS_NO_NULLPTR
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+struct _LIBCPP_TYPE_VIS_ONLY nullptr_t
+{
+void* __lx;
+
+struct __nat {int __for_bool_;};
+
+_LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR nullptr_t() : __lx(0) {}
+_LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR nullptr_t(int __nat::*) : __lx(0) 
{}
+
+_LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR operator int __nat::*() const 
{return 0;}
+
+template 
+_LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR
+operator _Tp* () const {return 0;}
+
+template 
+_LIBCPP_ALWAYS_INLINE
+operator _Tp _Up::* () const {return 0;}
+
+friend _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR bool operator==(nullptr_t, 
nullptr_t) {return true;}
+friend _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR bool operator!=(nullptr_t, 
nullptr_t) {return false;}
+friend _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR bool operator<(nullptr_t, 
nullptr_t) {return false;}
+friend _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR bool operator<=(nullptr_t, 
nullptr_t) {return true;}
+friend _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR bool operator>(nullptr_t, 
nullptr_t) {return false;}
+friend _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR bool operator>=(nullptr_t, 
nullptr_t) {return true;}
+};
+
+inline _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR nullptr_t __get_nullptr_t() 
{return nullptr_t(0);}
+
+#define nullptr _VSTD::__get_nullptr_t()
+
+_LIBCPP_END_NAMESPACE_STD
+
+#else  // _LIBCPP_HAS_NO_NULLPTR
+
+namespace std
+{
+typedef decltype(nullptr) nullptr_t;
+}
+
+#endif  // _LIBCPP_HAS_NO_NULLPTR
+
+#endif  // _LIBCPP_NULLPTR
diff --git include/cstddef include/cstddef
index c3ca64a..68f52c2 100644
--- include/cstddef
+++ include/cstddef
@@ -34,6 +34,7 @@ Types:
 */
 
 #include <__config>
+#include <__nullptr>
 
 #include 
 
@@ -53,50 +54,6 @@ using ::max_align_t;
 typedef long double max_align_t;
 #endif
 
-#ifdef _LIBCPP_HAS_NO_NULLPTR
-
-struct _LIBCPP_TYPE_VIS_ONLY nullptr_t
-{
-void* __lx;
-
-struct __nat {int __for_bool_;};
-
-_LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR nullptr_t() : __lx(0) {}
-_LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR nullptr_t(int __nat::*) : __lx(0) 
{}
-
-_LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR operator int __nat::*() const 
{return 0;}
-
-template 
-_LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR
-operator _Tp* () const {return 0;}
-
-template 
-_LIBCPP_ALWAYS_INLINE
-operator _Tp _Up::* () const {return 0;}
-
-friend _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR bool operator==(nullptr_t, 
nullptr_t) {return true;}
-friend _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR bool operator!=(nullptr_t, 
nullptr_t) {return false;}
-friend _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR bool operator<(nullptr_t, 
nullptr_t) {return false;}
-friend _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR bool operator<=(nullptr_t, 
nullptr_t) {return true;}
-friend 

Re: r249395 - BasicTests: Suppress InMemoryFileSystemTest.WindowsPath on win32 while investigating.

[NAKAMURA Takumi via cfe-commits]

On Wed, Oct 7, 2015 at 12:29 AM Rafael Espíndola 
wrote:

> What was the error?
>

http://lab.llvm.org:8011/builders/clang-x64-ninja-win7/builds/5683
http://bb.pgr.jp/builders/ninja-clang-i686-msc18-R/builds/3409
 http://bb.pgr.jp/builders/ninja-clang-x64-mingw64-RA/builds/8047

I guessed behavior difference in sys::Path iterator between unix and win32,
but not sure.



> On 6 October 2015 at 08:16, NAKAMURA Takumi via cfe-commits
>  wrote:
> > Author: chapuni
> > Date: Tue Oct  6 07:16:27 2015
> > New Revision: 249395
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=249395=rev
> > Log:
> > BasicTests: Suppress InMemoryFileSystemTest.WindowsPath on win32 while
> investigating.
> >
> > Modified:
> > cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
> >
> > Modified: cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp?rev=249395=249394=249395=diff
> >
> ==
> > --- cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp (original)
> > +++ cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp Tue Oct  6
> 07:16:27 2015
> > @@ -536,7 +536,9 @@ TEST_F(InMemoryFileSystemTest, IsEmpty)
> >  TEST_F(InMemoryFileSystemTest, WindowsPath) {
> >FS.addFile("c:/windows/system128/foo.cpp", 0,
> MemoryBuffer::getMemBuffer(""));
> >auto Stat = FS.status("c:");
> > +#if !defined(_WIN32)
> >ASSERT_FALSE(Stat.getError()) << Stat.getError() << FS.toString();
> > +#endif
> >Stat = FS.status("c:/windows/system128/foo.cpp");
> >ASSERT_FALSE(Stat.getError()) << Stat.getError() << FS.toString();
> >FS.addFile("d:/windows/foo.cpp", 0, MemoryBuffer::getMemBuffer(""));
> >
> >
> > ___
> > cfe-commits mailing list
> > cfe-commits@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12747: Implement [depr.c.headers]

[Richard Smith via cfe-commits]

. This one is tricky:

1) There's an (undocumented) interface between the C standard library and
this header, where the macros __need_ptrdiff_t, __need_size_t,
__need_wchar_t, __need_NULL, __need_wint_t request just a piece of this
header rather than the whole thing. If we see any of those, just go
straight to the underlying header.

2) We probably don't want  to include  (for consistency
with other headers), but  must provide a ::nullptr_t (which we
don't want  to provide). So neither header includes the other.
Instead, both include <__nullptr> for std::nullptr_t, and we duplicate the
definition of max_align_t between them, in the case where the compiler's
 doesn't provide it.

If you prefer, I could make  include  to avoid the
duplication of the max_align_t logic.

This patch depends on patch 02.

On Tue, Oct 6, 2015 at 3:42 PM, Richard Smith  wrote:

> , an easy one. We guarantee a setjmp macro exists even if this
> header is somehow included from C (the C standard allows that, so it's not
> worth checking for __cplusplus).
>
> On Tue, Oct 6, 2015 at 3:36 PM, Richard Smith 
> wrote:
>
>> Split  out of . This is a big change, but the same pattern
>> as the prior ones.
>>
>> On Tue, Oct 6, 2015 at 3:28 PM, Richard Smith 
>> wrote:
>>
>>> Likewise for , , 
>>>
>>> On Tue, Oct 6, 2015 at 3:20 PM, Richard Smith 
>>> wrote:
>>>
 Split  header out of 

 On Tue, Oct 6, 2015 at 3:07 PM, Richard Smith 
 wrote:

> Next: factoring the definition of std::nullptr_t out into a separate
> file, so that  and  can both use it, without 
> including  and without  providing a ::nullptr_t like
>  does.
>
> On Tue, Oct 6, 2015 at 3:02 PM, Eric Fiselier  wrote:
>
>> LGTM.
>>
>> On Tue, Oct 6, 2015 at 3:58 PM, Richard Smith 
>> wrote:
>> > On Mon, Oct 5, 2015 at 7:10 PM, Eric Fiselier  wrote:
>> >>
>> >> EricWF added a comment.
>> >>
>> >> I think thing change will help us close a number out outstanding
>> bugs. I
>> >> don't have any fundamental objections to this approach.  However
>> the size of
>> >> this patch scares me.  I understand the changes are mostly
>> mechanical but
>> >> their size can hide things. For example has anybody noticed that
>> your
>> >> internal changes to `` are in this patch? It would be nice
>> to break
>> >> this down into more digestible pieces that could be quickly spot
>> checked.
>> >
>> >
>> > OK. First such patch is attached. It just removes the
>> macro-capturing
>> > wrapper functions.
>>
>
>

>>>
>>
>
diff --git include/cstddef include/cstddef
index 68f52c2..1210b91 100644
--- include/cstddef
+++ include/cstddef
@@ -34,10 +34,10 @@ Types:
 */
 
 #include <__config>
+// Don't include our own ; we don't want to declare ::nullptr_t.
+#include_next 
 #include <__nullptr>
 
-#include 
-
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
 #endif
diff --git include/stddef.h include/stddef.h
new file mode 100644
index 000..6ffe582
--- /dev/null
+++ include/stddef.h
@@ -0,0 +1,56 @@
+// -*- C++ -*-
+//===--- stddef.h 
-===//
+//
+// 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.
+//
+//===--===//
+
+#if defined(__need_ptrdiff_t) || defined(__need_size_t) || \
+defined(__need_wchar_t) || defined(__need_NULL) || defined(__need_wint_t)
+#include_next 
+
+#elif !defined(_LIBCPP_STDDEF_H)
+#define _LIBCPP_STDDEF_H
+
+/*
+stddef.h synopsis
+
+Macros:
+
+offsetof(type,member-designator)
+NULL
+
+Types:
+
+ptrdiff_t
+size_t
+max_align_t
+nullptr_t
+
+*/
+
+#include <__config>
+#include_next 
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#pragma GCC system_header
+#endif
+
+#ifdef __cplusplus
+
+extern "C++" {
+#include <__nullptr>
+using std::nullptr_t;
+}
+
+// Re-use the compiler's  max_align_t where possible.
+#if !defined(__CLANG_MAX_ALIGN_T_DEFINED) && !defined(_GCC_MAX_ALIGN_T)
+typedef long double max_align_t;
+#endif
+
+#endif
+
+#endif  // _LIBCPP_STDDEF_H
diff --git test/std/depr/depr.c.headers/stddef_h.pass.cpp 
test/std/depr/depr.c.headers/stddef_h.pass.cpp
index 140c91b..c03c314 100644
--- test/std/depr/depr.c.headers/stddef_h.pass.cpp
+++ test/std/depr/depr.c.headers/stddef_h.pass.cpp
@@ -10,6 +10,7 @@
 // 
 
 #include 
+#include 
 #include 
 
 #ifndef NULL
@@ -22,6 +23,9 @@
 
 int main()
 {
+void *p = NULL;
+assert(!p);
+
 static_assert(sizeof(size_t) == sizeof(void*),
   "sizeof(size_t) == sizeof(void*)");
  

Re: [PATCH] D12747: Implement [depr.c.headers]

[Sean Silva via cfe-commits]

On Tue, Oct 6, 2015 at 4:13 PM, Richard Smith  wrote:

> On Tue, Oct 6, 2015 at 4:11 PM, Sean Silva  wrote:
>
>> +extern "C++" {
>> +#include <__nullptr>
>> +using std::nullptr_t;
>> +}
>>
>> Does this even compile with modules?
>>
>
> Yes. You're allowed to import a C++ module within an 'extern "C++"' block.
>

Why do we even need `extern "C++"` if we are already under `#ifdef
__cplusplus`?

-- Sean Silva


>
>
>> On Tue, Oct 6, 2015 at 3:57 PM, Richard Smith via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> . This one is tricky:
>>>
>>> 1) There's an (undocumented) interface between the C standard library
>>> and this header, where the macros __need_ptrdiff_t, __need_size_t,
>>> __need_wchar_t, __need_NULL, __need_wint_t request just a piece of this
>>> header rather than the whole thing. If we see any of those, just go
>>> straight to the underlying header.
>>>
>>> 2) We probably don't want  to include  (for
>>> consistency with other headers), but  must provide a ::nullptr_t
>>> (which we don't want  to provide). So neither header includes the
>>> other. Instead, both include <__nullptr> for std::nullptr_t, and we
>>> duplicate the definition of max_align_t between them, in the case where the
>>> compiler's  doesn't provide it.
>>>
>>> If you prefer, I could make  include  to avoid the
>>> duplication of the max_align_t logic.
>>>
>>> This patch depends on patch 02.
>>>
>>> On Tue, Oct 6, 2015 at 3:42 PM, Richard Smith 
>>> wrote:
>>>
 , an easy one. We guarantee a setjmp macro exists even if
 this header is somehow included from C (the C standard allows that, so it's
 not worth checking for __cplusplus).

 On Tue, Oct 6, 2015 at 3:36 PM, Richard Smith 
 wrote:

> Split  out of . This is a big change, but the same
> pattern as the prior ones.
>
> On Tue, Oct 6, 2015 at 3:28 PM, Richard Smith 
> wrote:
>
>> Likewise for , , 
>>
>> On Tue, Oct 6, 2015 at 3:20 PM, Richard Smith 
>> wrote:
>>
>>> Split  header out of 
>>>
>>> On Tue, Oct 6, 2015 at 3:07 PM, Richard Smith >> > wrote:
>>>
 Next: factoring the definition of std::nullptr_t out into a
 separate file, so that  and  can both use it, 
 without
  including  and without  providing a
 ::nullptr_t like  does.

 On Tue, Oct 6, 2015 at 3:02 PM, Eric Fiselier  wrote:

> LGTM.
>
> On Tue, Oct 6, 2015 at 3:58 PM, Richard Smith <
> rich...@metafoo.co.uk> wrote:
> > On Mon, Oct 5, 2015 at 7:10 PM, Eric Fiselier 
> wrote:
> >>
> >> EricWF added a comment.
> >>
> >> I think thing change will help us close a number out
> outstanding bugs. I
> >> don't have any fundamental objections to this approach.
> However the size of
> >> this patch scares me.  I understand the changes are mostly
> mechanical but
> >> their size can hide things. For example has anybody noticed
> that your
> >> internal changes to `` are in this patch? It would be
> nice to break
> >> this down into more digestible pieces that could be quickly
> spot checked.
> >
> >
> > OK. First such patch is attached. It just removes the
> macro-capturing
> > wrapper functions.
>


>>>
>>
>

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

r249484 - Fix Clang-tidy modernize-use-nullptr warnings in source directories; other minor cleanups

[Hans Wennborg via cfe-commits]

Author: hans
Date: Tue Oct  6 18:40:43 2015
New Revision: 249484

URL: http://llvm.org/viewvc/llvm-project?rev=249484=rev
Log:
Fix Clang-tidy modernize-use-nullptr warnings in source directories; other 
minor cleanups

Patch by Eugene Zelenko!

Differential Revision: http://reviews.llvm.org/D13406

Modified:
cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp
cfe/trunk/lib/CodeGen/CGBlocks.cpp
cfe/trunk/lib/CodeGen/CGClass.cpp
cfe/trunk/lib/CodeGen/CGException.cpp
cfe/trunk/lib/CodeGen/CGObjCMac.cpp
cfe/trunk/lib/CodeGen/CodeGenPGO.cpp
cfe/trunk/lib/Driver/ToolChains.cpp
cfe/trunk/lib/Frontend/ASTUnit.cpp
cfe/trunk/lib/Parse/ParseDecl.cpp
cfe/trunk/lib/Parse/ParseDeclCXX.cpp
cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
cfe/trunk/lib/Sema/SemaDeclObjC.cpp
cfe/trunk/lib/Sema/SemaLookup.cpp
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
cfe/trunk/tools/libclang/CXLoadedDiagnostic.cpp
cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
cfe/trunk/unittests/CodeGen/BufferSourceTest.cpp
cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp

Modified: cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp?rev=249484=249483=249484=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp Tue Oct  6 18:40:43 2015
@@ -1,4 +1,4 @@
-//===- ThreadSafetyCommon.cpp --*- C++ 
--*-===//
+//===- ThreadSafetyCommon.cpp ---*- C++ 
-*-===//
 //
 // The LLVM Compiler Infrastructure
 //
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+
 using namespace clang;
 using namespace threadSafety;
 
@@ -66,7 +67,6 @@ static bool isIncompletePhi(const til::S
 
 typedef SExprBuilder::CallingContext CallingContext;
 
-
 til::SExpr *SExprBuilder::lookupStmt(const Stmt *S) {
   auto It = SMap.find(S);
   if (It != SMap.end())
@@ -74,7 +74,6 @@ til::SExpr *SExprBuilder::lookupStmt(con
   return nullptr;
 }
 
-
 til::SCFG *SExprBuilder::buildCFG(CFGWalker ) {
   Walker.walk(*this);
   return Scfg;
@@ -85,7 +84,6 @@ static bool isCalleeArrow(const Expr *E)
   return ME ? ME->isArrow() : false;
 }
 
-
 /// \brief Translate a clang expression in an attribute to a til::SExpr.
 /// Constructs the context from D, DeclExp, and SelfDecl.
 ///
@@ -148,7 +146,6 @@ CapabilityExpr SExprBuilder::translateAt
 return translateAttrExpr(AttrExp, );
 }
 
-
 /// \brief Translate a clang expression in an attribute to a til::SExpr.
 // This assumes a CallingContext has already been created.
 CapabilityExpr SExprBuilder::translateAttrExpr(const Expr *AttrExp,
@@ -195,8 +192,6 @@ CapabilityExpr SExprBuilder::translateAt
   return CapabilityExpr(E, Neg);
 }
 
-
-
 // Translate a clang statement or expression to a TIL expression.
 // Also performs substitution of variables; Ctx provides the context.
 // Dispatches on the type of S.
@@ -268,8 +263,6 @@ til::SExpr *SExprBuilder::translate(cons
   return new (Arena) til::Undefined(S);
 }
 
-
-
 til::SExpr *SExprBuilder::translateDeclRefExpr(const DeclRefExpr *DRE,
CallingContext *Ctx) {
   const ValueDecl *VD = cast(DRE->getDecl()->getCanonicalDecl());
@@ -294,7 +287,6 @@ til::SExpr *SExprBuilder::translateDeclR
   return new (Arena) til::LiteralPtr(VD);
 }
 
-
 til::SExpr *SExprBuilder::translateCXXThisExpr(const CXXThisExpr *TE,
CallingContext *Ctx) {
   // Substitute for 'this'
@@ -313,7 +305,7 @@ static const ValueDecl *getValueDeclFrom
 return P->clangDecl();
   if (auto *L = dyn_cast(E))
 return L->clangDecl();
-  return 0;
+  return nullptr;
 }
 
 static bool hasCppPointerType(const til::SExpr *E) {
@@ -355,7 +347,6 @@ til::SExpr *SExprBuilder::translateMembe
   return P;
 }
 
-
 til::SExpr *SExprBuilder::translateCallExpr(const CallExpr *CE,
 CallingContext *Ctx,
 const Expr *SelfE) {
@@ -381,7 +372,6 @@ til::SExpr *SExprBuilder::translateCallE
   return new (Arena) til::Call(E, CE);
 }
 
-
 til::SExpr *SExprBuilder::translateCXXMemberCallExpr(
 const CXXMemberCallExpr *ME, CallingContext *Ctx) {
   if (CapabilityExprMode) {
@@ -397,7 +387,6 @@ til::SExpr *SExprBuilder::translateCXXMe
ME->getImplicitObjectArgument());
 }
 
-
 til::SExpr *SExprBuilder::translateCXXOperatorCallExpr(
 const CXXOperatorCallExpr *OCE, CallingContext *Ctx) {
   if (CapabilityExprMode) {
@@ -412,7 +401,6 @@ til::SExpr *SExprBuilder::translateCXXOp
   return translateCallExpr(cast(OCE), Ctx);
 }
 
-
 til::SExpr *SExprBuilder::translateUnaryOperator(const UnaryOperator *UO,
 

Re: [PATCH] D13406: [Clang] Fix Clang-tidy modernize-use-nullptr warnings in source directories; other minor cleanups.

[Hans Wennborg via cfe-commits]

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

lgtm



Comment at: lib/Sema/SemaDeclObjC.cpp:510
@@ -509,3 +509,3 @@
 
-if (PrevDecl && SuperClassDecl == 0) {
+if (PrevDecl && (!SuperClassDecl)) {
   // The previous declaration was not a class decl. Check if we have a

I don't think the extra parentheses are customary here.


Repository:
  rL LLVM

http://reviews.llvm.org/D13406



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

Re: [PATCH] D13407: [libcxx] Capture configuration information when installing the libc++ headers

[Eric Fiselier via cfe-commits]

EricWF updated this revision to Diff 36682.
EricWF added a comment.

Use `type` instead of `cat` on windows as suggested by @rnk. @eugenis does this 
address your concern?


http://reviews.llvm.org/D13407

Files:
  CMakeLists.txt
  cmake/Modules/HandleLibcxxFlags.cmake
  docs/DesignDocs/CapturingConfigInfo.rst
  docs/index.rst
  include/CMakeLists.txt
  include/__config_site.in

Index: include/__config_site.in
===
--- /dev/null
+++ include/__config_site.in
@@ -0,0 +1,20 @@
+//===--===//
+//
+// 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_CONFIG_SITE
+#define _LIBCPP_CONFIG_SITE
+
+#cmakedefine _LIBCPP_HAS_NO_GLOBAL_FILESYSTEM_NAMESPACE
+#cmakedefine _LIBCPP_HAS_NO_STDIN
+#cmakedefine _LIBCPP_HAS_NO_STDOUT
+#cmakedefine _LIBCPP_HAS_NO_THREADS
+#cmakedefine _LIBCPP_HAS_NO_MONOTONIC_CLOCK
+#cmakedefine _LIBCPP_HAS_NO_THREAD_UNSAFE_C_FUNCTIONS
+
+#endif
Index: include/CMakeLists.txt
===
--- include/CMakeLists.txt
+++ include/CMakeLists.txt
@@ -1,10 +1,12 @@
 if (NOT LIBCXX_INSTALL_SUPPORT_HEADERS)
   set(LIBCXX_SUPPORT_HEADER_PATTERN PATTERN "support" EXCLUDE)
 endif()
+
 set(LIBCXX_HEADER_PATTERN
   PATTERN "*"
   PATTERN "CMakeLists.txt" EXCLUDE
   PATTERN ".svn" EXCLUDE
+  PATTERN "__config_site.in" EXCLUDE
   ${LIBCXX_SUPPORT_HEADER_PATTERN}
   )
 
@@ -22,4 +24,28 @@
 ${LIBCXX_HEADER_PATTERN}
 PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ
   )
+
+  if (LIBCXX_NEEDS_SITE_CONFIG)
+set(UNIX_CAT cat)
+if (WIN32)
+  set(UNIX_CAT type)
+endif()
+# Generate and install a custom __config header. The new header is created
+# by  prepending __config_site to the current __config header.
+add_custom_command(OUTPUT ${LIBCXX_BINARY_DIR}/__generated_config
+  COMMAND ${CMAKE_COMMAND} -E copy ${LIBCXX_BINARY_DIR}/__config_site ${LIBCXX_BINARY_DIR}/__generated_config
+  COMMAND ${UNIX_CAT} ${LIBCXX_SOURCE_DIR}/include/__config >> ${LIBCXX_BINARY_DIR}/__generated_config
+  DEPENDS ${LIBCXX_SOURCE_DIR}/include/__config
+)
+# Add a target that executes the generation commands.
+add_custom_target(generate_config_header ALL
+  DEPENDS ${LIBCXX_BINARY_DIR}/__generated_config)
+# Install the generated header as __config.
+install(FILES ${LIBCXX_BINARY_DIR}/__generated_config
+  DESTINATION include/c++/v1
+  PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ
+  RENAME __config
+  COMPONENT libcxx)
+  endif()
+
 endif()
Index: docs/index.rst
===
--- docs/index.rst
+++ docs/index.rst
@@ -124,6 +124,12 @@
 Design Documents
 
 
+.. toctree::
+   :maxdepth: 1
+
+   DesignDocs/CapturingConfigInfo
+
+
 * ` design `_
 * ` design `_
 * `Status of debug mode `_
Index: docs/DesignDocs/CapturingConfigInfo.rst
===
--- /dev/null
+++ docs/DesignDocs/CapturingConfigInfo.rst
@@ -0,0 +1,88 @@
+===
+Capturing configuration information during installation
+===
+
+.. contents::
+   :local:
+
+The Problem
+===
+
+Currently the libc++ supports building the library with a number of different
+configuration options.  Unfortunately all of that configuration information is
+lost when libc++ is installed. In order to support "persistent"
+configurations libc++ needs a mechanism to capture the configuration options
+in the INSTALLED headers.
+
+
+Design Goals
+
+
+* The solution should not INSTALL any additional headers. We don't want an extra
+  #include slowing everybody down.
+
+* The solution should not unduly affect libc++ developers. The problem is limited
+  to installed versions of libc++ and the solution should be as well.
+
+* The solution should not modify any existing headers EXCEPT during installation.
+  It makes developers lives harder if they have to regenerate the libc++ headers
+  every time they are modified.
+
+* The solution should not make any of the libc++ headers dependant on
+  files generated by the build system. The headers should be able to compile
+  out of the box without any modification.
+
+* The solution should not have ANY effect on users who don't need special
+  configuration options. The vast majority of users will never need this so it
+  shouldn't cost them.
+
+
+The Solution
+

[PATCH] D13488: [analyzer] Assume escape is possible through system functions taking void*

[Anna Zaks via cfe-commits]

zaks.anna created this revision.
zaks.anna added a reviewer: dcoughlin.
zaks.anna added subscribers: xazax.hun, cfe-commits.
Herald added a subscriber: aemerson.

The analyzer assumes that system functions will not free memory or modify the 
arguments in other ways, so we assume that arguments do not escape when those 
are called. However, this may lead to false positive leak errors. For example, 
in code like this where the pointers added to the rb_tree are freed later on:
struct alarm_event *e = calloc(1, sizeof(*e));

rb_tree_insert_node(_tree, e);

Add a heuristic to assume that calls to system functions taking void* arguments 
allow for pointer escape.

http://reviews.llvm.org/D13488

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  lib/StaticAnalyzer/Core/CallEvent.cpp
  test/Analysis/Inputs/system-header-simulator.h
  test/Analysis/malloc.c

Index: test/Analysis/malloc.c
===
--- test/Analysis/malloc.c
+++ test/Analysis/malloc.c
@@ -1657,6 +1657,23 @@
   return p;
 }
 
+// Some data structures may hold onto the pointer and free it later.
+void testEscapeThroughSystemCallTakingVoidPointer1(void *queue) {
+  int *data = (int *)malloc(32);
+  fake_insque(queue, data); // no warning
+}
+
+void testEscapeThroughSystemCallTakingVoidPointer2(fake_rb_tree_t *rbt) {
+  int *data = (int *)malloc(32);
+  fake_rb_tree_init(rbt, data);
+} //expected-warning{{Potential leak}}
+
+void testEscapeThroughSystemCallTakingVoidPointer3(fake_rb_tree_t *rbt) {
+  int *data = (int *)malloc(32);
+  fake_rb_tree_init(rbt, data);
+  fake_rb_tree_insert_node(rbt, data); // no warning
+}
+
 // 
 // False negatives.
 
Index: test/Analysis/Inputs/system-header-simulator.h
===
--- test/Analysis/Inputs/system-header-simulator.h
+++ test/Analysis/Inputs/system-header-simulator.h
@@ -82,6 +82,12 @@
 void fakeSystemHeaderCallInt(int *);
 void fakeSystemHeaderCallIntPtr(int **);
 
+// Some data strauctures may hold onto the pointer and free it later.
+void fake_insque(void *, void *);
+typedef struct fake_rb_tree { void *opaque[8]; } fake_rb_tree_t;
+void fake_rb_tree_init(fake_rb_tree_t *, const void *);
+void *fake_rb_tree_insert_node(fake_rb_tree_t *, void *);
+
 typedef struct __SomeStruct {
   char * p;
 } SomeStruct;
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -50,11 +50,7 @@
   return ResultTy;
 }
 
-static bool isCallbackArg(SVal V, QualType T) {
-  // If the parameter is 0, it's harmless.
-  if (V.isZeroConstant())
-return false;
-
+static bool isCallback(QualType T) {
   // If a parameter is a block or a callback, assume it can modify pointer.
   if (T->isBlockPointerType() ||
   T->isFunctionPointerType() ||
@@ -75,32 +71,53 @@
 return true;
 }
   }
-
   return false;
 }
 
-bool CallEvent::hasNonZeroCallbackArg() const {
+static bool isVoidPointerToNonConst(QualType T) {
+  if (const PointerType *PT = T->getAs()) {
+QualType PointeeTy = PT->getPointeeType();
+if (PointeeTy.isConstQualified())
+  return false;
+return PointeeTy->isVoidType();
+  } else
+return false;
+}
+
+bool CallEvent::hasNonNullArgumentsWithType(bool (*Condition)(QualType)) const {
   unsigned NumOfArgs = getNumArgs();
 
   // If calling using a function pointer, assume the function does not
-  // have a callback. TODO: We could check the types of the arguments here.
+  // satisfy the callback.
+  // TODO: We could check the types of the arguments here.
   if (!getDecl())
 return false;
 
   unsigned Idx = 0;
   for (CallEvent::param_type_iterator I = param_type_begin(),
-   E = param_type_end();
+  E = param_type_end();
I != E && Idx < NumOfArgs; ++I, ++Idx) {
 if (NumOfArgs <= Idx)
   break;
 
-if (isCallbackArg(getArgSVal(Idx), *I))
+// If the parameter is 0, it's harmless.
+if (getArgSVal(Idx).isZeroConstant())
+  continue;
+
+if (Condition(*I))
   return true;
   }
-
   return false;
 }
 
+bool CallEvent::hasNonZeroCallbackArg() const {
+  return hasNonNullArgumentsWithType(isCallback);
+}
+
+bool CallEvent::hasVoidPointerToNonConstArg() const {
+  return hasNonNullArgumentsWithType(isVoidPointerToNonConst);
+}
+
 bool CallEvent::isGlobalCFunction(StringRef FunctionName) const {
   const FunctionDecl *FD = dyn_cast_or_null(getDecl());
   if (!FD)
@@ -326,7 +343,7 @@
 }
 
 bool AnyFunctionCall::argumentsMayEscape() const {
-  if (hasNonZeroCallbackArg())
+  if (CallEvent::argumentsMayEscape() || hasVoidPointerToNonConstArg())
 return true;
 

r249388 - [VFS] Port applyAllReplacements to InMemoryFileSystem.

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Tue Oct  6 05:23:17 2015
New Revision: 249388

URL: http://llvm.org/viewvc/llvm-project?rev=249388=rev
Log:
[VFS] Port applyAllReplacements to InMemoryFileSystem.

Modified:
cfe/trunk/lib/Tooling/Core/Replacement.cpp

Modified: cfe/trunk/lib/Tooling/Core/Replacement.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Core/Replacement.cpp?rev=249388=249387=249388=diff
==
--- cfe/trunk/lib/Tooling/Core/Replacement.cpp (original)
+++ cfe/trunk/lib/Tooling/Core/Replacement.cpp Tue Oct  6 05:23:17 2015
@@ -265,19 +265,18 @@ bool applyAllReplacements(const std::vec
 }
 
 std::string applyAllReplacements(StringRef Code, const Replacements ) 
{
-  FileManager Files((FileSystemOptions()));
+  IntrusiveRefCntPtr InMemoryFileSystem(
+  new vfs::InMemoryFileSystem);
+  FileManager Files(FileSystemOptions(), InMemoryFileSystem);
   DiagnosticsEngine Diagnostics(
   IntrusiveRefCntPtr(new DiagnosticIDs),
   new DiagnosticOptions);
   SourceManager SourceMgr(Diagnostics, Files);
   Rewriter Rewrite(SourceMgr, LangOptions());
-  std::unique_ptr Buf =
-  llvm::MemoryBuffer::getMemBuffer(Code, "");
-  const clang::FileEntry *Entry =
-  Files.getVirtualFile("", Buf->getBufferSize(), 0);
-  SourceMgr.overrideFileContents(Entry, std::move(Buf));
-  FileID ID =
-  SourceMgr.createFileID(Entry, SourceLocation(), clang::SrcMgr::C_User);
+  InMemoryFileSystem->addFile(
+  "", 0, llvm::MemoryBuffer::getMemBuffer(Code, ""));
+  FileID ID = SourceMgr.createFileID(Files.getFile(""), 
SourceLocation(),
+ clang::SrcMgr::C_User);
   for (Replacements::const_iterator I = Replaces.begin(), E = Replaces.end();
I != E; ++I) {
 Replacement Replace("", I->getOffset(), I->getLength(),


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

r249389 - [VFS] Port SimpleFormatContext to InMemoryFileSystem.

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Tue Oct  6 05:23:34 2015
New Revision: 249389

URL: http://llvm.org/viewvc/llvm-project?rev=249389=rev
Log:
[VFS] Port SimpleFormatContext to InMemoryFileSystem.

Modified:
cfe/trunk/lib/Index/SimpleFormatContext.h

Modified: cfe/trunk/lib/Index/SimpleFormatContext.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/SimpleFormatContext.h?rev=249389=249388=249389=diff
==
--- cfe/trunk/lib/Index/SimpleFormatContext.h (original)
+++ cfe/trunk/lib/Index/SimpleFormatContext.h Tue Oct  6 05:23:34 2015
@@ -38,18 +38,17 @@ public:
   : DiagOpts(new DiagnosticOptions()),
 Diagnostics(new DiagnosticsEngine(new DiagnosticIDs,
   DiagOpts.get())),
-Files((FileSystemOptions())),
+InMemoryFileSystem(new vfs::InMemoryFileSystem),
+Files(FileSystemOptions(), InMemoryFileSystem),
 Sources(*Diagnostics, Files),
 Rewrite(Sources, Options) {
 Diagnostics->setClient(new IgnoringDiagConsumer, true);
   }
 
   FileID createInMemoryFile(StringRef Name, StringRef Content) {
-std::unique_ptr Source =
-llvm::MemoryBuffer::getMemBuffer(Content);
-const FileEntry *Entry =
-Files.getVirtualFile(Name, Source->getBufferSize(), 0);
-Sources.overrideFileContents(Entry, std::move(Source));
+InMemoryFileSystem->addFile(Name, 0,
+llvm::MemoryBuffer::getMemBuffer(Content));
+const FileEntry *Entry = Files.getFile(Name);
 assert(Entry != nullptr);
 return Sources.createFileID(Entry, SourceLocation(), SrcMgr::C_User);
   }
@@ -64,6 +63,7 @@ public:
 
   IntrusiveRefCntPtr DiagOpts;
   IntrusiveRefCntPtr Diagnostics;
+  IntrusiveRefCntPtr InMemoryFileSystem;
   FileManager Files;
   SourceManager Sources;
   Rewriter Rewrite;


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

r249391 - Adds a way for tools to deduce the target config from a compiler name.

[Manuel Klimek via cfe-commits]

Author: klimek
Date: Tue Oct  6 05:45:03 2015
New Revision: 249391

URL: http://llvm.org/viewvc/llvm-project?rev=249391=rev
Log:
Adds a way for tools to deduce the target config from a compiler name.

Adds `addTargetAndModeForProgramName`, a utility function that will add
appropriate `-target foo` and `--driver-mode=g++` tokens to a command
line for driver invocations of the form `a/b/foo-g++`. It is intended to
support tooling: for example, should a compilation database record some
invocation of `foo-g++` without these implicit flags, a Clang tool may
use this function to add them back.

Patch by Luke Zarko.

Modified:
cfe/trunk/include/clang/Tooling/Tooling.h
cfe/trunk/lib/Tooling/Tooling.cpp
cfe/trunk/unittests/Tooling/CMakeLists.txt
cfe/trunk/unittests/Tooling/ToolingTest.cpp

Modified: cfe/trunk/include/clang/Tooling/Tooling.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Tooling.h?rev=249391=249390=249391=diff
==
--- cfe/trunk/include/clang/Tooling/Tooling.h (original)
+++ cfe/trunk/include/clang/Tooling/Tooling.h Tue Oct  6 05:45:03 2015
@@ -417,6 +417,29 @@ inline std::unique_ptr ,
+StringRef InvokedAs);
+
 /// \brief Creates a \c CompilerInvocation.
 clang::CompilerInvocation *newInvocation(
 clang::DiagnosticsEngine *Diagnostics,

Modified: cfe/trunk/lib/Tooling/Tooling.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Tooling.cpp?rev=249391=249390=249391=diff
==
--- cfe/trunk/lib/Tooling/Tooling.cpp (original)
+++ cfe/trunk/lib/Tooling/Tooling.cpp Tue Oct  6 05:45:03 2015
@@ -17,6 +17,7 @@
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
 #include "clang/Driver/Tool.h"
+#include "clang/Driver/ToolChain.h"
 #include "clang/Frontend/ASTUnit.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
@@ -162,6 +163,31 @@ std::string getAbsolutePath(StringRef Fi
   return AbsolutePath.str();
 }
 
+void addTargetAndModeForProgramName(std::vector ,
+StringRef InvokedAs) {
+  if (!CommandLine.empty() && !InvokedAs.empty()) {
+bool AlreadyHasTarget = false;
+bool AlreadyHasMode = false;
+// Skip CommandLine[0].
+for (auto Token = ++CommandLine.begin(); Token != CommandLine.end();
+ ++Token) {
+  StringRef TokenRef(*Token);
+  AlreadyHasTarget |=
+  (TokenRef == "-target" || TokenRef.startswith("-target="));
+  AlreadyHasMode |= (TokenRef == "--driver-mode" ||
+ TokenRef.startswith("--driver-mode="));
+}
+auto TargetMode =
+clang::driver::ToolChain::getTargetAndModeFromProgramName(InvokedAs);
+if (!AlreadyHasMode && !TargetMode.second.empty()) {
+  CommandLine.insert(++CommandLine.begin(), TargetMode.second);
+}
+if (!AlreadyHasTarget && !TargetMode.first.empty()) {
+  CommandLine.insert(++CommandLine.begin(), {"-target", TargetMode.first});
+}
+  }
+}
+
 namespace {
 
 class SingleFrontendActionFactory : public FrontendActionFactory {

Modified: cfe/trunk/unittests/Tooling/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/CMakeLists.txt?rev=249391=249390=249391=diff
==
--- cfe/trunk/unittests/Tooling/CMakeLists.txt (original)
+++ cfe/trunk/unittests/Tooling/CMakeLists.txt Tue Oct  6 05:45:03 2015
@@ -1,4 +1,5 @@
 set(LLVM_LINK_COMPONENTS
+  ${LLVM_TARGETS_TO_BUILD}
   Support
   )
 

Modified: cfe/trunk/unittests/Tooling/ToolingTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/ToolingTest.cpp?rev=249391=249390=249391=diff
==
--- cfe/trunk/unittests/Tooling/ToolingTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/ToolingTest.cpp Tue Oct  6 05:45:03 2015
@@ -18,6 +18,8 @@
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Config/llvm-config.h"
+#include "llvm/Support/TargetSelect.h"
+#include "llvm/Support/TargetRegistry.h"
 #include "gtest/gtest.h"
 #include 
 #include 
@@ -291,6 +293,84 @@ TEST(ClangToolTest, ArgumentAdjusters) {
   EXPECT_FALSE(Found);
 }
 
+namespace {
+/// Find a target name such that looking for it in TargetRegistry by that name
+/// returns the same target. We expect that there is at least one target
+/// configured with this property.
+std::string getAnyTarget() {
+  llvm::InitializeAllTargets();
+  for (const auto  : llvm::TargetRegistry::targets()) {
+std::string Error;
+if (llvm::TargetRegistry::lookupTarget(Target.getName(), Error) ==
+) {
+  return Target.getName();
+}
+  }
+  return "";
+}
+}
+
+TEST(addTargetAndModeForProgramName, AddsTargetAndMode) {

Re: [PATCH] D13318: Add a utility function to add target information to a command line

[Manuel Klimek via cfe-commits]

klimek closed this revision.
klimek added a comment.

Submitted as r249391.


http://reviews.llvm.org/D13318



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

r249392 - clang-format: Make IncludeCategories configurable in .clang-format file.

[Daniel Jasper via cfe-commits]

Author: djasper
Date: Tue Oct  6 06:54:18 2015
New Revision: 249392

URL: http://llvm.org/viewvc/llvm-project?rev=249392=rev
Log:
clang-format: Make IncludeCategories configurable in .clang-format file.

This was made much easier by introducing an IncludeCategory struct to
replace the previously used std::pair.

Also, cleaned up documentation and added examples.

Modified:
cfe/trunk/docs/ClangFormatStyleOptions.rst
cfe/trunk/docs/tools/dump_format_style.py
cfe/trunk/include/clang/Format/Format.h
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/docs/ClangFormatStyleOptions.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangFormatStyleOptions.rst?rev=249392=249391=249392=diff
==
--- cfe/trunk/docs/ClangFormatStyleOptions.rst (original)
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst Tue Oct  6 06:54:18 2015
@@ -155,21 +155,29 @@ the configuration (without a prefix: ``A
 
   This applies to round brackets (parentheses), angle brackets and square
   brackets. This will result in formattings like
-  \code
-  someLongFunction(argument1,
-  argument2);
-  \endcode
+  .. code-block:: c++
+someLongFunction(argument1,
+ argument2);
 
 **AlignConsecutiveAssignments** (``bool``)
   If ``true``, aligns consecutive assignments.
 
   This will align the assignment operators of consecutive lines. This
   will result in formattings like
-  \code
-  int  = 12;
-  int b= 23;
-  int ccc  = 23;
-  \endcode
+  .. code-block:: c++
+int  = 12;
+int b= 23;
+int ccc  = 23;
+
+**AlignConsecutiveDeclarations** (``bool``)
+  If ``true``, aligns consecutive declarations.
+
+  This will align the declaration names of consecutive lines. This
+  will result in formattings like
+  .. code-block:: c++
+int  = 12;
+float   b = 23;
+std::string ccc = 23;
 
 **AlignEscapedNewlinesLeft** (``bool``)
   If ``true``, aligns escaped newlines as far left as possible.
@@ -381,14 +389,17 @@ the configuration (without a prefix: ``A
   instead of as function calls.
 
   These are expected to be macros of the form:
-  \code
-  FOREACH(, ...)
-  
-  \endcode
+  .. code-block:: c++
+FOREACH(, ...)
+  
+
+  In the .clang-format configuration file, this can be configured like:
+  .. code-block:: c++
+ForEachMacros: ['RANGES_FOR', 'FOREACH']
 
   For example: BOOST_FOREACH.
 
-**IncludeCategories** (``std::vector>``)
+**IncludeCategories** (``std::vector``)
   Regular expressions denoting the different #include categories used
   for ordering #includes.
 
@@ -403,6 +414,16 @@ the configuration (without a prefix: ``A
   so that it is kept at the beginning of the #includes
   (http://llvm.org/docs/CodingStandards.html#include-style).
 
+  To configure this in the .clang-format file, use:
+  .. code-block:: c++
+IncludeCategories:
+  - Regex:   '^"(llvm|llvm-c|clang|clang-c)/'
+Priority:2
+  - Regex:   '^(<|"(gtest|isl|json)/)'
+Priority:3
+  - Regex:   '.\*'
+Priority:1
+
 **IndentCaseLabels** (``bool``)
   Indent case labels one level from the switch statement.
 

Modified: cfe/trunk/docs/tools/dump_format_style.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/tools/dump_format_style.py?rev=249392=249391=249392=diff
==
--- cfe/trunk/docs/tools/dump_format_style.py (original)
+++ cfe/trunk/docs/tools/dump_format_style.py Tue Oct  6 06:54:18 2015
@@ -86,7 +86,11 @@ class EnumValue:
 doxygen2rst(indent(self.comment, 2)))
 
 def clean_comment_line(line):
-  return line[3:].strip() + '\n'
+  if line == '/// \\code':
+return '.. code-block:: c++\n'
+  if line == '/// \\endcode':
+return ''
+  return line[4:] + '\n'
 
 def read_options(header):
   class State:
@@ -139,8 +143,6 @@ def read_options(header):
   elif line == '};':
 state = State.InStruct
 nested_structs[nested_struct.name] = nested_struct
-  else:
-raise Exception('Invalid format, expected struct field comment or };')
 elif state == State.InNestedFieldComent:
   if line.startswith('///'):
 comment += clean_comment_line(line)
@@ -168,7 +170,7 @@ def read_options(header):
   for option in options:
 if not option.type in ['bool', 'unsigned', 'int', 'std::string',
'std::vector',
-   'std::vector>']:
+   'std::vector']:
   if enums.has_key(option.type):
 option.enum = enums[option.type]
   elif nested_structs.has_key(option.type):

Modified: cfe/trunk/include/clang/Format/Format.h
URL: 

Re: [PATCH] D13469: Create interfaces and tests for the overlapping replacements fix in clang-tidy.

[Angel Garcia via cfe-commits]

angelgarcia updated this revision to Diff 36620.
angelgarcia added a comment.

Explain why we have defined these mocks.


http://reviews.llvm.org/D13469

Files:
  unittests/clang-tidy/CMakeLists.txt
  unittests/clang-tidy/ClangTidyTest.h
  unittests/clang-tidy/OverlappingReplacementsTest.cpp
  unittests/clang-tidy/ReadabilityModuleTest.cpp

Index: unittests/clang-tidy/ReadabilityModuleTest.cpp
===
--- unittests/clang-tidy/ReadabilityModuleTest.cpp
+++ unittests/clang-tidy/ReadabilityModuleTest.cpp
@@ -237,7 +237,7 @@
 
 TEST(BracesAroundStatementsCheck, IfElseWithShortStatements) {
   ClangTidyOptions Options;
-  Options.CheckOptions["test-check.ShortStatementLines"] = "1";
+  Options.CheckOptions["test-check-0.ShortStatementLines"] = "1";
 
   EXPECT_EQ("int main() {\n"
 "  if (true) return 1;\n"
Index: unittests/clang-tidy/OverlappingReplacementsTest.cpp
===
--- /dev/null
+++ unittests/clang-tidy/OverlappingReplacementsTest.cpp
@@ -0,0 +1,379 @@
+//=== OverlappingReplacementsTest.cpp - clang-tidy ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "ClangTidyTest.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace tidy {
+namespace test {
+namespace {
+
+const char BoundDecl[] = "decl";
+const char BoundIf[] = "if";
+
+// We define a reduced set of very small checks that allow to test different
+// overlapping situations (no overlapping, replacements partially overlap, etc),
+// as well as different kinds of diagnostics (one check produces several errors,
+// several replacement ranges in an error, etc).
+class UseCharCheck : public ClangTidyCheck {
+public:
+  UseCharCheck(StringRef CheckName, ClangTidyContext *Context)
+  : ClangTidyCheck(CheckName, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override {
+using namespace ast_matchers;
+Finder->addMatcher(varDecl(hasType(isInteger())).bind(BoundDecl), this);
+  }
+  void check(const ast_matchers::MatchFinder::MatchResult ) override {
+auto *VD = Result.Nodes.getNodeAs(BoundDecl);
+diag(VD->getLocStart(), "use char") << FixItHint::CreateReplacement(
+CharSourceRange::getTokenRange(VD->getLocStart(), VD->getLocStart()),
+"char");
+  }
+};
+
+class IfFalseCheck : public ClangTidyCheck {
+public:
+  IfFalseCheck(StringRef CheckName, ClangTidyContext *Context)
+  : ClangTidyCheck(CheckName, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override {
+using namespace ast_matchers;
+Finder->addMatcher(ifStmt().bind(BoundIf), this);
+  }
+  void check(const ast_matchers::MatchFinder::MatchResult ) override {
+auto *If = Result.Nodes.getNodeAs(BoundIf);
+auto *Cond = If->getCond();
+SourceRange Range = Cond->getSourceRange();
+if (auto *D = If->getConditionVariable()) {
+  Range = SourceRange(D->getLocStart(), D->getLocEnd());
+}
+diag(Range.getBegin(), "the cake is a lie") << FixItHint::CreateReplacement(
+CharSourceRange::getTokenRange(Range), "false");
+  }
+};
+
+class RefactorCheck : public ClangTidyCheck {
+public:
+  RefactorCheck(StringRef CheckName, ClangTidyContext *Context)
+  : ClangTidyCheck(CheckName, Context), NamePattern("::$") {}
+  RefactorCheck(StringRef CheckName, ClangTidyContext *Context,
+StringRef NamePattern)
+  : ClangTidyCheck(CheckName, Context), NamePattern(NamePattern) {}
+  virtual std::string newName(StringRef OldName) = 0;
+
+  void registerMatchers(ast_matchers::MatchFinder *Finder) final {
+using namespace ast_matchers;
+Finder->addMatcher(varDecl(matchesName(NamePattern)).bind(BoundDecl), this);
+  }
+
+  void check(const ast_matchers::MatchFinder::MatchResult ) final {
+auto *VD = Result.Nodes.getNodeAs(BoundDecl);
+std::string NewName = newName(VD->getName());
+
+auto Diag = diag(VD->getLocation(), "refactor")
+<< FixItHint::CreateReplacement(
+CharSourceRange::getTokenRange(VD->getLocation(),
+   VD->getLocation()),
+NewName);
+
+class UsageVisitor : public RecursiveASTVisitor {
+public:
+  UsageVisitor(const ValueDecl *VD, StringRef NewName,
+   DiagnosticBuilder )
+  : VD(VD), NewName(NewName), Diag(Diag) {}
+  bool VisitDeclRefExpr(DeclRefExpr *E) {
+if (const ValueDecl *D = E->getDecl()) {
+  if (VD->getCanonicalDecl() == D->getCanonicalDecl()) {
+Diag << FixItHint::CreateReplacement(
+

Re: [PATCH] D13311: [clang-tidy] Add check cppcoreguidelines-pro-bounds-pointer-arithmetic

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.


Comment at: 
test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic.cpp:37
@@ +36,3 @@
+  q -= i;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use pointer arithmetic
+  q -= ENUM_LITERAL;

I don't think this comment is "done" yet. I still don't know how this check is 
intended to handle code like that. Does it currently diagnose? Does it not 
diagnose? Should it diagnose?


Comment at: 
test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic.cpp:51
@@ +50,3 @@
+
+  i = p[1];
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use pointer arithmetic

How well does this handle code like:
```
void f(int i[], size_t s) {
  i[s - 1] = 0;
}
```
Does it diagnose, and should it?


http://reviews.llvm.org/D13311



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

Re: [PATCH] D13383: [clang] Add flag to DeclContext to distinguish between qualified and unqualified name lookups

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

I'm uncertain whether this change is good or not (Richard is more likely to 
have thoughts on that), but the patch is missing tests.



Comment at: clang/lib/Sema/SemaLookup.cpp:208
@@ +207,3 @@
+
+  class Deinitializer {
+std::function Deinit;

This should be a local class defined more closely to its usage.


http://reviews.llvm.org/D13383



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

Re: [PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

In http://reviews.llvm.org/D13368#260672, @klimek wrote:

> In http://reviews.llvm.org/D13368#260669, @aaron.ballman wrote:
>
> > This wasn't a comment on the rule so much as a comment on the diagnostic 
> > not being very helpful.In this case, you're telling the user to not do 
> > something, but it is unclear how the user would structure their code to 
> > silence this diagnostic. Perhaps there is no way to word this to give the 
> > user a clue, but we should at least try. If I got this diagnostic as it is 
> > now, I would scratch my head and quickly decide to ignore it.
>
>
> The cpp core guidelines are written in a way that they should be 
> referenceable by links - do we want to add links to the relevant portions of 
> the core guidelines from the clang-tidy checks?


I'd be hesitant to do that. It would add a lot of verbiage to diagnostics that 
are likely to be chatty, and if the links ever go dead mid-release cycle for 
us, we're stuck looking bad with no way to fix it. CERT's guidelines are also 
linkable in the same fashion (as would be hypothetical checks for MISRA, JSF, 
etc), and I would have the same hesitation for those as well due to the 
potential dead link issue.

I think that having the links within the user-facing documentation is a 
must-have though (and something we've been pretty good about thus far) because 
those can be updated live from ToT. So perhaps a permanent short link to our 
own documentation might be useful (if a bit obtuse since our docs mostly just 
point to other docs elsewhere)? I'd still be a bit worried about expired short 
links or something, but maybe we already host a service for this sort of thing?


http://reviews.llvm.org/D13368



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

Re: [PATCH] D13313: [clang-tidy] new check cppcoreguidelines-pro-type-reinterpret-cast

[Aaron Ballman via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

Thank you for the patch! I've committed it in r249399.

~Aaron


http://reviews.llvm.org/D13313



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

r249395 - BasicTests: Suppress InMemoryFileSystemTest.WindowsPath on win32 while investigating.

[NAKAMURA Takumi via cfe-commits]

Author: chapuni
Date: Tue Oct  6 07:16:27 2015
New Revision: 249395

URL: http://llvm.org/viewvc/llvm-project?rev=249395=rev
Log:
BasicTests: Suppress InMemoryFileSystemTest.WindowsPath on win32 while 
investigating.

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

Modified: cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp?rev=249395=249394=249395=diff
==
--- cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp (original)
+++ cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp Tue Oct  6 07:16:27 2015
@@ -536,7 +536,9 @@ TEST_F(InMemoryFileSystemTest, IsEmpty)
 TEST_F(InMemoryFileSystemTest, WindowsPath) {
   FS.addFile("c:/windows/system128/foo.cpp", 0, 
MemoryBuffer::getMemBuffer(""));
   auto Stat = FS.status("c:");
+#if !defined(_WIN32)
   ASSERT_FALSE(Stat.getError()) << Stat.getError() << FS.toString();
+#endif
   Stat = FS.status("c:/windows/system128/foo.cpp");
   ASSERT_FALSE(Stat.getError()) << Stat.getError() << FS.toString();
   FS.addFile("d:/windows/foo.cpp", 0, MemoryBuffer::getMemBuffer(""));


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

Re: [PATCH] D13383: [clang] Add flag to DeclContext to distinguish between qualified and unqualified name lookups

[Eugene Leviant via cfe-commits]

evgeny777 added a comment.

Hello Jim and Sean,

Greg suggested adding you for this review. Can you please take a look?


http://reviews.llvm.org/D13383



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

Re: [PATCH] D13386: PR24115: Don't instantiate constexpr function templates in decltype

[Stephan Bergmann via cfe-commits]

sberg abandoned this revision.
sberg added a comment.

Ah, I see.  I'll wait for that approach to be implemented then.


http://reviews.llvm.org/D13386



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

r249403 - ToolingTests: Tweak getAnyTarget() to match "x86_64".

[NAKAMURA Takumi via cfe-commits]

Author: chapuni
Date: Tue Oct  6 08:58:13 2015
New Revision: 249403

URL: http://llvm.org/viewvc/llvm-project?rev=249403=rev
Log:
ToolingTests: Tweak getAnyTarget() to match "x86_64".

Both "x86" and "x86-64" are incompatible to triple's arch.

Modified:
cfe/trunk/unittests/Tooling/ToolingTest.cpp

Modified: cfe/trunk/unittests/Tooling/ToolingTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/ToolingTest.cpp?rev=249403=249402=249403=diff
==
--- cfe/trunk/unittests/Tooling/ToolingTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/ToolingTest.cpp Tue Oct  6 08:58:13 2015
@@ -301,9 +301,11 @@ std::string getAnyTarget() {
   llvm::InitializeAllTargets();
   for (const auto  : llvm::TargetRegistry::targets()) {
 std::string Error;
-if (llvm::TargetRegistry::lookupTarget(Target.getName(), Error) ==
-) {
-  return Target.getName();
+StringRef TargetName(Target.getName());
+if (TargetName == "x86-64")
+  TargetName = "x86_64";
+if (llvm::TargetRegistry::lookupTarget(TargetName, Error) == ) {
+  return TargetName;
 }
   }
   return "";


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

[clang-tools-extra] r249399 - Add a new module for the C++ Core Guidelines, and the first checker for those guidelines: cppcoreguidelines-pro-type-reinterpret-cast.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Tue Oct  6 08:31:00 2015
New Revision: 249399

URL: http://llvm.org/viewvc/llvm-project?rev=249399=rev
Log:
Add a new module for the C++ Core Guidelines, and the first checker for those 
guidelines: cppcoreguidelines-pro-type-reinterpret-cast.

Patch by Matthias Gehre!

Added:
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt

clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/Makefile

clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeReinterpretCastCheck.cpp

clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeReinterpretCastCheck.h

clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-type-reinterpret-cast.rst

clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-reinterpret-cast.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/Makefile
clang-tools-extra/trunk/clang-tidy/add_new_check.py
clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/tool/ClangTidyMain.cpp
clang-tools-extra/trunk/clang-tidy/tool/Makefile
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/CMakeLists.txt?rev=249399=249398=249399=diff
==
--- clang-tools-extra/trunk/clang-tidy/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/CMakeLists.txt Tue Oct  6 08:31:00 2015
@@ -28,6 +28,7 @@ add_clang_library(clangTidy
 add_subdirectory(tool)
 add_subdirectory(cert)
 add_subdirectory(llvm)
+add_subdirectory(cppcoreguidelines)
 add_subdirectory(google)
 add_subdirectory(misc)
 add_subdirectory(modernize)

Modified: clang-tools-extra/trunk/clang-tidy/Makefile
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/Makefile?rev=249399=249398=249399=diff
==
--- clang-tools-extra/trunk/clang-tidy/Makefile (original)
+++ clang-tools-extra/trunk/clang-tidy/Makefile Tue Oct  6 08:31:00 2015
@@ -11,6 +11,6 @@ CLANG_LEVEL := ../../..
 LIBRARYNAME := clangTidy
 include $(CLANG_LEVEL)/../../Makefile.config
 
-DIRS = utils cert readability llvm google misc modernize tool
+DIRS = utils cert cppcoreguidelines readability llvm google misc modernize tool
 
 include $(CLANG_LEVEL)/Makefile

Modified: clang-tools-extra/trunk/clang-tidy/add_new_check.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/add_new_check.py?rev=249399=249398=249399=diff
==
--- clang-tools-extra/trunk/clang-tidy/add_new_check.py (original)
+++ clang-tools-extra/trunk/clang-tidy/add_new_check.py Tue Oct  6 08:31:00 2015
@@ -147,7 +147,8 @@ void %(check_name)s::check(const MatchFi
 
 # Modifies the module to include the new check.
 def adapt_module(module_path, module, check_name, check_name_camel):
-  filename = os.path.join(module_path, module.capitalize() + 'TidyModule.cpp')
+  modulecpp = filter(lambda p: p.lower() == module.lower() + "tidymodule.cpp", 
os.listdir(module_path))[0]
+  filename = os.path.join(module_path, modulecpp)
   with open(filename, 'r') as f:
 lines = f.readlines()
 

Added: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt?rev=249399=auto
==
--- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt (added)
+++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt Tue Oct 
 6 08:31:00 2015
@@ -0,0 +1,15 @@
+set(LLVM_LINK_COMPONENTS support)
+
+add_clang_library(clangTidyCppCoreGuidelinesModule
+  CppCoreGuidelinesTidyModule.cpp
+  ProTypeReinterpretCastCheck.cpp
+
+  LINK_LIBS
+  clangAST
+  clangASTMatchers
+  clangBasic
+  clangLex
+  clangTidy
+  clangTidyUtils
+  clangTooling
+  )

Added: 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp?rev=249399=auto
==
--- 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
 (added)
+++ 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
 Tue Oct  6 08:31:00 2015
@@ -0,0 +1,38 @@
+//===--- CppCoreGuidelinesModule.cpp - clang-tidy 
-===//
+//
+// 

[clang-tools-extra] r249402 - Create interfaces and tests for the overlapping replacements fix in clang-tidy.

[Angel Garcia Gomez via cfe-commits]

Author: angelgarcia
Date: Tue Oct  6 08:52:51 2015
New Revision: 249402

URL: http://llvm.org/viewvc/llvm-project?rev=249402=rev
Log:
Create interfaces and tests for the overlapping replacements fix in clang-tidy.

Summary: No changes in clang-tidy yet.

Reviewers: klimek

Subscribers: alexfh, cfe-commits

Differential Revision: http://reviews.llvm.org/D13469

Added:
clang-tools-extra/trunk/unittests/clang-tidy/OverlappingReplacementsTest.cpp
Modified:
clang-tools-extra/trunk/unittests/clang-tidy/CMakeLists.txt
clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h
clang-tools-extra/trunk/unittests/clang-tidy/ReadabilityModuleTest.cpp

Modified: clang-tools-extra/trunk/unittests/clang-tidy/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/CMakeLists.txt?rev=249402=249401=249402=diff
==
--- clang-tools-extra/trunk/unittests/clang-tidy/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/unittests/clang-tidy/CMakeLists.txt Tue Oct  6 
08:52:51 2015
@@ -13,6 +13,7 @@ add_extra_unittest(ClangTidyTests
   GoogleModuleTest.cpp
   LLVMModuleTest.cpp
   MiscModuleTest.cpp
+  OverlappingReplacementsTest.cpp
   ReadabilityModuleTest.cpp)
 
 target_link_libraries(ClangTidyTests

Modified: clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h?rev=249402=249401=249402=diff
==
--- clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h (original)
+++ clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h Tue Oct  6 
08:52:51 2015
@@ -18,6 +18,7 @@
 #include "clang/Tooling/Refactoring.h"
 #include "clang/Tooling/Tooling.h"
 #include 
+#include 
 
 namespace clang {
 namespace tidy {
@@ -25,9 +26,10 @@ namespace test {
 
 class TestClangTidyAction : public ASTFrontendAction {
 public:
-  TestClangTidyAction(ClangTidyCheck , ast_matchers::MatchFinder ,
+  TestClangTidyAction(SmallVectorImpl ,
+  ast_matchers::MatchFinder ,
   ClangTidyContext )
-  : Check(Check), Finder(Finder), Context(Context) {}
+  : Checks(Checks), Finder(Finder), Context(Context) {}
 
 private:
   std::unique_ptr CreateASTConsumer(CompilerInstance ,
@@ -36,17 +38,37 @@ private:
 Context.setCurrentFile(File);
 Context.setASTContext(());
 
-Check.registerMatchers();
-Check.registerPPCallbacks(Compiler);
+for (auto  : Checks) {
+  Check->registerMatchers();
+  Check->registerPPCallbacks(Compiler);
+}
 return Finder.newASTConsumer();
   }
 
-  ClangTidyCheck 
+  SmallVectorImpl 
   ast_matchers::MatchFinder 
   ClangTidyContext 
 };
 
-template 
+template  struct CheckFactory {
+  static void
+  createChecks(ClangTidyContext *Context,
+   SmallVectorImpl ) {
+CheckFactory::createChecks(Context, Result);
+CheckFactory::createChecks(Context, Result);
+  }
+};
+
+template  struct CheckFactory {
+  static void
+  createChecks(ClangTidyContext *Context,
+   SmallVectorImpl ) {
+Result.emplace_back(llvm::make_unique(
+"test-check-" + std::to_string(Result.size()), Context));
+  }
+};
+
+template 
 std::string
 runCheckOnCode(StringRef Code, std::vector *Errors = nullptr,
const Twine  = "input.cc",
@@ -59,7 +81,6 @@ runCheckOnCode(StringRef Code, std::vect
   ClangTidyContext Context(llvm::make_unique(
   ClangTidyGlobalOptions(), Options));
   ClangTidyDiagnosticConsumer DiagConsumer(Context);
-  T Check("test-check", );
 
   std::vector ArgCXX11(1, "clang-tidy");
   ArgCXX11.push_back("-fsyntax-only");
@@ -71,8 +92,11 @@ runCheckOnCode(StringRef Code, std::vect
   ast_matchers::MatchFinder Finder;
   llvm::IntrusiveRefCntPtr Files(
   new FileManager(FileSystemOptions()));
+
+  SmallVector Checks;
+  CheckFactory::createChecks(, Checks);
   tooling::ToolInvocation Invocation(
-  ArgCXX11, new TestClangTidyAction(Check, Finder, Context), Files.get());
+  ArgCXX11, new TestClangTidyAction(Checks, Finder, Context), Files.get());
   Invocation.mapVirtualFile(Filename.str(), Code);
   for (const auto  : PathsToContent) {
 Invocation.mapVirtualFile(Twine("include/" + FileContent.first).str(),

Added: 
clang-tools-extra/trunk/unittests/clang-tidy/OverlappingReplacementsTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/OverlappingReplacementsTest.cpp?rev=249402=auto
==
--- 
clang-tools-extra/trunk/unittests/clang-tidy/OverlappingReplacementsTest.cpp 
(added)
+++ 
clang-tools-extra/trunk/unittests/clang-tidy/OverlappingReplacementsTest.cpp 
Tue Oct  6 08:52:51 

Re: [PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

[Manuel Klimek via cfe-commits]

klimek added a subscriber: klimek.
klimek added a comment.

In http://reviews.llvm.org/D13368#260669, @aaron.ballman wrote:

> This wasn't a comment on the rule so much as a comment on the diagnostic not 
> being very helpful.In this case, you're telling the user to not do something, 
> but it is unclear how the user would structure their code to silence this 
> diagnostic. Perhaps there is no way to word this to give the user a clue, but 
> we should at least try. If I got this diagnostic as it is now, I would 
> scratch my head and quickly decide to ignore it.


The cpp core guidelines are written in a way that they should be referenceable 
by links - do we want to add links to the relevant portions of the core 
guidelines from the clang-tidy checks?


http://reviews.llvm.org/D13368



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

r249394 - clang-format: Add empty line before code-blocks in Docs.

[Daniel Jasper via cfe-commits]

Author: djasper
Date: Tue Oct  6 07:11:51 2015
New Revision: 249394

URL: http://llvm.org/viewvc/llvm-project?rev=249394=rev
Log:
clang-format: Add empty line before code-blocks in Docs.

Modified:
cfe/trunk/docs/ClangFormatStyleOptions.rst
cfe/trunk/docs/tools/dump_format_style.py

Modified: cfe/trunk/docs/ClangFormatStyleOptions.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangFormatStyleOptions.rst?rev=249394=249393=249394=diff
==
--- cfe/trunk/docs/ClangFormatStyleOptions.rst (original)
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst Tue Oct  6 07:11:51 2015
@@ -155,6 +155,7 @@ the configuration (without a prefix: ``A
 
   This applies to round brackets (parentheses), angle brackets and square
   brackets. This will result in formattings like
+
   .. code-block:: c++
 someLongFunction(argument1,
  argument2);
@@ -164,6 +165,7 @@ the configuration (without a prefix: ``A
 
   This will align the assignment operators of consecutive lines. This
   will result in formattings like
+
   .. code-block:: c++
 int  = 12;
 int b= 23;
@@ -174,6 +176,7 @@ the configuration (without a prefix: ``A
 
   This will align the declaration names of consecutive lines. This
   will result in formattings like
+
   .. code-block:: c++
 int  = 12;
 float   b = 23;
@@ -389,11 +392,13 @@ the configuration (without a prefix: ``A
   instead of as function calls.
 
   These are expected to be macros of the form:
+
   .. code-block:: c++
 FOREACH(, ...)
   
 
   In the .clang-format configuration file, this can be configured like:
+
   .. code-block:: c++
 ForEachMacros: ['RANGES_FOR', 'FOREACH']
 
@@ -415,6 +420,7 @@ the configuration (without a prefix: ``A
   (http://llvm.org/docs/CodingStandards.html#include-style).
 
   To configure this in the .clang-format file, use:
+
   .. code-block:: c++
 IncludeCategories:
   - Regex:   '^"(llvm|llvm-c|clang|clang-c)/'

Modified: cfe/trunk/docs/tools/dump_format_style.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/tools/dump_format_style.py?rev=249394=249393=249394=diff
==
--- cfe/trunk/docs/tools/dump_format_style.py (original)
+++ cfe/trunk/docs/tools/dump_format_style.py Tue Oct  6 07:11:51 2015
@@ -87,7 +87,7 @@ class EnumValue:
 
 def clean_comment_line(line):
   if line == '/// \\code':
-return '.. code-block:: c++\n'
+return '\n.. code-block:: c++\n'
   if line == '/// \\endcode':
 return ''
   return line[4:] + '\n'


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

Re: [PATCH] D13469: Create interfaces and tests for the overlapping replacements fix in clang-tidy.

[Manuel Klimek via cfe-commits]

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

LG



Comment at: unittests/clang-tidy/OverlappingReplacementsTest.cpp:21
@@ +20,3 @@
+const char BoundIf[] = "if";
+
+class UseCharCheck : public ClangTidyCheck {

I'd add a comment here that we're defining a couple of checks that help us 
check different variants of overlapping results.


http://reviews.llvm.org/D13469



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

[PATCH] D13469: Create interfaces and tests for the overlapping replacements fix in clang-tidy.

[Angel Garcia via cfe-commits]

angelgarcia created this revision.
angelgarcia added a reviewer: klimek.
angelgarcia added subscribers: cfe-commits, alexfh.

No changes in clang-tidy yet.

http://reviews.llvm.org/D13469

Files:
  unittests/clang-tidy/CMakeLists.txt
  unittests/clang-tidy/ClangTidyTest.h
  unittests/clang-tidy/OverlappingReplacementsTest.cpp
  unittests/clang-tidy/ReadabilityModuleTest.cpp

Index: unittests/clang-tidy/ReadabilityModuleTest.cpp
===
--- unittests/clang-tidy/ReadabilityModuleTest.cpp
+++ unittests/clang-tidy/ReadabilityModuleTest.cpp
@@ -237,7 +237,7 @@
 
 TEST(BracesAroundStatementsCheck, IfElseWithShortStatements) {
   ClangTidyOptions Options;
-  Options.CheckOptions["test-check.ShortStatementLines"] = "1";
+  Options.CheckOptions["test-check-0.ShortStatementLines"] = "1";
 
   EXPECT_EQ("int main() {\n"
 "  if (true) return 1;\n"
Index: unittests/clang-tidy/OverlappingReplacementsTest.cpp
===
--- /dev/null
+++ unittests/clang-tidy/OverlappingReplacementsTest.cpp
@@ -0,0 +1,344 @@
+//=== OverlappingReplacementsTest.cpp - clang-tidy ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "ClangTidyTest.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace tidy {
+namespace test {
+namespace {
+
+const char BoundDecl[] = "decl";
+const char BoundIf[] = "if";
+
+class UseCharCheck : public ClangTidyCheck {
+public:
+  UseCharCheck(StringRef CheckName, ClangTidyContext *Context)
+  : ClangTidyCheck(CheckName, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override {
+using namespace ast_matchers;
+Finder->addMatcher(varDecl(hasType(isInteger())).bind(BoundDecl), this);
+  }
+  void check(const ast_matchers::MatchFinder::MatchResult ) override {
+auto *VD = Result.Nodes.getNodeAs(BoundDecl);
+diag(VD->getLocStart(), "use char") << FixItHint::CreateReplacement(
+CharSourceRange::getTokenRange(VD->getLocStart(), VD->getLocStart()),
+"char");
+  }
+};
+
+class IfFalseCheck : public ClangTidyCheck {
+public:
+  IfFalseCheck(StringRef CheckName, ClangTidyContext *Context)
+  : ClangTidyCheck(CheckName, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override {
+using namespace ast_matchers;
+Finder->addMatcher(ifStmt().bind(BoundIf), this);
+  }
+  void check(const ast_matchers::MatchFinder::MatchResult ) override {
+auto *If = Result.Nodes.getNodeAs(BoundIf);
+auto *Cond = If->getCond();
+SourceRange Range = Cond->getSourceRange();
+if (auto *D = If->getConditionVariable()) {
+  Range = SourceRange(D->getLocStart(), D->getLocEnd());
+}
+diag(Range.getBegin(), "the cake is a lie") << FixItHint::CreateReplacement(
+CharSourceRange::getTokenRange(Range), "false");
+  }
+};
+
+class RefactorCheck : public ClangTidyCheck {
+public:
+  RefactorCheck(StringRef CheckName, ClangTidyContext *Context)
+  : ClangTidyCheck(CheckName, Context), NamePattern("::$") {}
+  RefactorCheck(StringRef CheckName, ClangTidyContext *Context,
+StringRef NamePattern)
+  : ClangTidyCheck(CheckName, Context), NamePattern(NamePattern) {}
+  virtual std::string newName(StringRef OldName) = 0;
+
+  void registerMatchers(ast_matchers::MatchFinder *Finder) final {
+using namespace ast_matchers;
+Finder->addMatcher(varDecl(matchesName(NamePattern)).bind(BoundDecl), this);
+  }
+
+  void check(const ast_matchers::MatchFinder::MatchResult ) final {
+auto *VD = Result.Nodes.getNodeAs(BoundDecl);
+std::string NewName = newName(VD->getName());
+
+auto Diag = diag(VD->getLocation(), "refactor")
+<< FixItHint::CreateReplacement(
+CharSourceRange::getTokenRange(VD->getLocation(),
+   VD->getLocation()),
+NewName);
+
+class UsageVisitor : public RecursiveASTVisitor {
+public:
+  UsageVisitor(const ValueDecl *VD, StringRef NewName,
+   DiagnosticBuilder )
+  : VD(VD), NewName(NewName), Diag(Diag) {}
+  bool VisitDeclRefExpr(DeclRefExpr *E) {
+if (const ValueDecl *D = E->getDecl()) {
+  if (VD->getCanonicalDecl() == D->getCanonicalDecl()) {
+Diag << FixItHint::CreateReplacement(
+CharSourceRange::getTokenRange(E->getSourceRange()), NewName);
+  }
+}
+return RecursiveASTVisitor::VisitDeclRefExpr(E);
+  }
+
+private:
+  const ValueDecl *VD;
+  StringRef NewName;
+  DiagnosticBuilder 
+};
+
+UsageVisitor(VD, NewName, 

Re: [PATCH] D13474: [VFS] Port tooling to use the in-memory file system.

[Benjamin Kramer via cfe-commits]

bkramer updated this revision to Diff 36632.
bkramer added a comment.

- Don't rebuild VFS for every compile command
- Still have to guard against multiple runs of one Tool, 
ClangToolTest.ArgumentAdjusters does that.


http://reviews.llvm.org/D13474

Files:
  include/clang/Tooling/Tooling.h
  lib/Tooling/Tooling.cpp
  unittests/Tooling/ToolingTest.cpp

Index: unittests/Tooling/ToolingTest.cpp
===
--- unittests/Tooling/ToolingTest.cpp
+++ unittests/Tooling/ToolingTest.cpp
@@ -149,39 +149,54 @@
 }
 
 TEST(ToolInvocation, TestMapVirtualFile) {
-  IntrusiveRefCntPtr Files(
-  new clang::FileManager(clang::FileSystemOptions()));
+  llvm::IntrusiveRefCntPtr OverlayFileSystem(
+  new vfs::OverlayFileSystem(vfs::getRealFileSystem()));
+  llvm::IntrusiveRefCntPtr InMemoryFileSystem(
+  new vfs::InMemoryFileSystem);
+  OverlayFileSystem->pushOverlay(InMemoryFileSystem);
+  llvm::IntrusiveRefCntPtr Files(
+  new FileManager(FileSystemOptions(), OverlayFileSystem));
   std::vector Args;
   Args.push_back("tool-executable");
   Args.push_back("-Idef");
   Args.push_back("-fsyntax-only");
   Args.push_back("test.cpp");
   clang::tooling::ToolInvocation Invocation(Args, new SyntaxOnlyAction,
 Files.get());
-  Invocation.mapVirtualFile("test.cpp", "#include \n");
-  Invocation.mapVirtualFile("def/abc", "\n");
+  InMemoryFileSystem->addFile(
+  "test.cpp", 0, llvm::MemoryBuffer::getMemBuffer("#include \n"));
+  InMemoryFileSystem->addFile("def/abc", 0,
+  llvm::MemoryBuffer::getMemBuffer("\n"));
   EXPECT_TRUE(Invocation.run());
 }
 
 TEST(ToolInvocation, TestVirtualModulesCompilation) {
   // FIXME: Currently, this only tests that we don't exit with an error if a
   // mapped module.map is found on the include path. In the future, expand this
   // test to run a full modules enabled compilation, so we make sure we can
   // rerun modules compilations with a virtual file system.
-  IntrusiveRefCntPtr Files(
-  new clang::FileManager(clang::FileSystemOptions()));
+  llvm::IntrusiveRefCntPtr OverlayFileSystem(
+  new vfs::OverlayFileSystem(vfs::getRealFileSystem()));
+  llvm::IntrusiveRefCntPtr InMemoryFileSystem(
+  new vfs::InMemoryFileSystem);
+  OverlayFileSystem->pushOverlay(InMemoryFileSystem);
+  llvm::IntrusiveRefCntPtr Files(
+  new FileManager(FileSystemOptions(), OverlayFileSystem));
   std::vector Args;
   Args.push_back("tool-executable");
   Args.push_back("-Idef");
   Args.push_back("-fsyntax-only");
   Args.push_back("test.cpp");
   clang::tooling::ToolInvocation Invocation(Args, new SyntaxOnlyAction,
 Files.get());
-  Invocation.mapVirtualFile("test.cpp", "#include \n");
-  Invocation.mapVirtualFile("def/abc", "\n");
+  InMemoryFileSystem->addFile(
+  "test.cpp", 0, llvm::MemoryBuffer::getMemBuffer("#include \n"));
+  InMemoryFileSystem->addFile("def/abc", 0,
+  llvm::MemoryBuffer::getMemBuffer("\n"));
   // Add a module.map file in the include directory of our header, so we trigger
   // the module.map header search logic.
-  Invocation.mapVirtualFile("def/module.map", "\n");
+  InMemoryFileSystem->addFile("def/module.map", 0,
+  llvm::MemoryBuffer::getMemBuffer("\n"));
   EXPECT_TRUE(Invocation.run());
 }
 
Index: lib/Tooling/Tooling.cpp
===
--- lib/Tooling/Tooling.cpp
+++ lib/Tooling/Tooling.cpp
@@ -32,13 +32,6 @@
 #include "llvm/Support/Host.h"
 #include "llvm/Support/raw_ostream.h"
 
-// For chdir, see the comment in ClangTool::run for more information.
-#ifdef LLVM_ON_WIN32
-#  include 
-#else
-#  include 
-#endif
-
 #define DEBUG_TYPE "clang-tooling"
 
 namespace clang {
@@ -131,18 +124,25 @@
 
   SmallString<16> FileNameStorage;
   StringRef FileNameRef = FileName.toNullTerminatedStringRef(FileNameStorage);
+  llvm::IntrusiveRefCntPtr OverlayFileSystem(
+  new vfs::OverlayFileSystem(vfs::getRealFileSystem()));
+  llvm::IntrusiveRefCntPtr InMemoryFileSystem(
+  new vfs::InMemoryFileSystem);
+  OverlayFileSystem->pushOverlay(InMemoryFileSystem);
   llvm::IntrusiveRefCntPtr Files(
-  new FileManager(FileSystemOptions()));
+  new FileManager(FileSystemOptions(), OverlayFileSystem));
   ToolInvocation Invocation(getSyntaxOnlyToolArgs(Args, FileNameRef),
 ToolAction, Files.get(), PCHContainerOps);
 
   SmallString<1024> CodeStorage;
-  Invocation.mapVirtualFile(FileNameRef,
-Code.toNullTerminatedStringRef(CodeStorage));
+  InMemoryFileSystem->addFile(FileNameRef, 0,
+  llvm::MemoryBuffer::getMemBuffer(
+  Code.toNullTerminatedStringRef(CodeStorage)));
 
   for (auto  : VirtualMappedFiles) {
-

Re: [PATCH] D13465: Add SafeStack support for test-suite.

[Jonathan Roelofs via cfe-commits]

jroelofs added a subscriber: jroelofs.
jroelofs accepted this revision.
jroelofs added a reviewer: jroelofs.
jroelofs added a comment.
This revision is now accepted and ready to land.

LGTM


http://reviews.llvm.org/D13465



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

Re: [PATCH] D13465: Add SafeStack support for test-suite.

[Jonathan Roelofs via cfe-commits]

jroelofs added a comment.

In http://reviews.llvm.org/D13465#260628, @tharvik wrote:

> When running the test suite, it gives a bunch of `undefined reference to 
> '__safestack_unsafe_stack_ptr'`.
>  See regression results .


Sounds like it's missing the safestack runtime, which should be provided by 
building compiler-rt.


http://reviews.llvm.org/D13465



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

Re: [PATCH] D13383: [clang] Add flag to DeclContext to distinguish between qualified and unqualified name lookups

[Eugene Leviant via cfe-commits]

evgeny777 added a comment.

In case of qualified lookup it raises flag in DeclContext, which is not used 
anywhere else in clang, only in lldb (clang expression parser is linked to 
lldb).


http://reviews.llvm.org/D13383



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

Re: [PATCH] D12839: Extend MoveConstructorInitCheck to also flag constructor arguments passed by value and can be moved assigned to fields.

[Felix Berger via cfe-commits]

flx added a comment.

I don't have access, so that'd be great if you or Alex could submit the patch, 
thanks!



Comment at: clang-tidy/misc/MoveConstructorInitCheck.cpp:11
@@ -10,2 +10,3 @@
 #include "MoveConstructorInitCheck.h"
+#include "../utils/Matchers.h"
 #include "clang/AST/ASTContext.h"

aaron.ballman wrote:
> Errr, I'm not certain that we typically use relative paths for this sort of 
> thing. I see we do use relative paths for other things in clang-tidy, but 
> this is definitely an uncommon thing in the rest of the code base (outside of 
> test or example code).
> 
> Not saying you should change anything here, but we may want to consider 
> changing the paths at some point so we are more in line with the rest of the 
> LLVM projects in not using relative include paths.
Leaving this as is for now as all other ClangTidy specific includes I could 
find were relative as well. Not sure if non-relative includes require extra 
work in the build rules.


Comment at: clang-tidy/misc/MoveConstructorInitCheck.cpp:96
@@ +95,3 @@
+
+void MoveConstructorInitCheck::handleMoveConstructor(
+const MatchFinder::MatchResult ) {

alexfh wrote:
> Other checks have separate options for the include style (see 
> `PassByValueCheck.cpp`, for example), so this one should have its own option 
> too. If we ever decide to introduce a common option instead, it will be 
> easier to do this, if all usages of the `IncludeInserter` do the same thing 
> with the include style.
> 
> One thing I would change though, is to add the IncludeStyle <-> string 
> conversion functions instead of doing this in each check.
Done. I moved the conversion into IncludeSorter where the the enum is defined.


Comment at: clang-tidy/modernize/PassByValueCheck.cpp:121
@@ -120,4 +120,3 @@
 : ClangTidyCheck(Name, Context),
-  IncludeStyle(Options.get("IncludeStyle", "llvm") == "llvm" ?
-   IncludeSorter::IS_LLVM : IncludeSorter::IS_Google) {}
+  IncludeStyle(IncludeSorter::toIncludeStyle(Options.get("IncludeStyle", 
"llvm"))) {}
 

alexfh wrote:
> clang-format?
This is how clang-format formatted it. I used the following invocation:

clang-format --style=LLVM MoveConstructorInitCheck.cpp  > formatted

And then diffed to only update format changes in code I added.


Comment at: clang-tidy/utils/IncludeSorter.h:29
@@ +28,3 @@
+  // Converts "llvm" to IS_LLVM, otherwise returns IS_Google.
+  static IncludeStyle toIncludeStyle(const std::string );
+

alexfh wrote:
> nit: The current name gives no hints at what is being converted to 
> IncludeStyle, but `parseIncludeStyle` would make it clear that the source is 
> a string.
Done, although the argument type uniquely identifies a function.


Comment at: clang-tidy/utils/Matchers.h:1
@@ +1,2 @@
+//===--- Matchers.h - 
clang-tidy---===//
+//

aaron.ballman wrote:
> You are missing header include guards.
Thanks for catching that. Done.


Comment at: clang-tidy/utils/TypeTraits.cpp:32
@@ +31,3 @@
+  // Ignore trivially copyable types.
+  return !(Type->isScalarType() || Type.isTriviallyCopyableType(Context) ||
+   classHasTrivialCopyAndDestroy(Type));

aaron.ballman wrote:
> No need to check for isScalarType() because isTriviallyCopyableType() already 
> does that correctly.
Done. Also demorganized the expression to make it easier to understand.


Comment at: clang-tidy/utils/TypeTraits.h:22
@@ +21,3 @@
+/// destructor.
+bool classHasTrivialCopyAndDestroy(QualType Type);
+

aaron.ballman wrote:
> I think this is an implementation detail more than a trait we want to expose.
Removed from header.


http://reviews.llvm.org/D12839



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

Re: [PATCH] D12839: Extend MoveConstructorInitCheck to also flag constructor arguments passed by value and can be moved assigned to fields.

[Aaron Ballman via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

Thank you for the patch! I've commit in r249429.


http://reviews.llvm.org/D12839



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

Re: [PATCH] D13383: [clang] Add flag to DeclContext to distinguish between qualified and unqualified name lookups

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

In http://reviews.llvm.org/D13383#260815, @evgeny777 wrote:

> What kind of tests do you propose for this? It doesn't introduce any new 
> features to clang, only to lldb where the separate review exists


It modifies the behavior of all qualified lookups in Clang, so I would assume 
there would be some behavior change associated with this?


http://reviews.llvm.org/D13383



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

Re: [PATCH] D13383: [clang] Add flag to DeclContext to distinguish between qualified and unqualified name lookups

[Eugene Leviant via cfe-commits]

evgeny777 added a comment.

What kind of tests do you propose for this? It doesn't introduce any new 
features to clang, only to lldb where the separate review exists


http://reviews.llvm.org/D13383



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