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 &TC,
> - const ArgList &Args) {
> -  const llvm::Triple &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 &TC) {
> -  // The runtimes are located in the OS-specific resource directory.
> -  SmallString<128> Res(TC.getDriver().ResourceDir);
> -  const llvm::Triple &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 &TC, const ArgList 
> &Args,
> -  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 &TC,
> -  const llvm::opt::ArgList &Args,
> -  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 &TC, const ArgList &Args,
> ArgStringList &CmdArgs) {
> -  CmdArgs.push_back(getCompilerRTArgString(TC, Args, "builtins"));
> -}
> -
> -static void addProfileRT(const ToolChain &TC, const ArgList &Args,
> - ArgStringList &CmdArgs) {
> -  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 &TC, const Ar

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] D13099: [Analyzer] Don’t invalidate CXXThis when conservatively evaluating const methods (PR 21606)

[Sean Eveson via cfe-commits]

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

Updated to latest revision.
Moved tests into their own file.
Added (RegionAndSymbolInvalidationTraits *ETraits) parameter to 
CallEvent::getExtraInvalidatedValues and overrides.
Prevent invalidation by using TK_PreserveContents (Which also fixes the pointer 
issue).
Simplified the patch so it falls back on invalidating the entire object if 
there is a mutable field, since invalidating a single field invalidated the 
entire object anyway.


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,94 @@
+// 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 {
+  mutable int mut;
+  void foo() const;
+};
+
+struct Outer;
+
+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 = &x;
+  t.foo();
+  clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatConstMethodDoesInvalidateInheritedMutableFields() {
+  Derived t;
+  t.b_mut = 4;
+  t.mut = 4;
+  t.foo();
+  clang_analyzer_eval(t.b_mut); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(t.mut); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatConstMethodDoesInvalidateInheritedPointedAtMemory() {
+  int x = 1;
+  Derived t;
+  t.p = &x;
+  t.foo();
+  clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatConstMethodDoesInvalidateContainedPointedAtMemory() {
+  int x = 1;
+  Outer t;
+  t.x = 2;
+  t.in.p = &x;
+  t.foo();
+  clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(t.x == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(t.in.p == &x); // 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, &ETraits);
 
   // 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 &Values) const {
-  Values.push_back(getCXXThisVal());
+void CXXInstanceCall::getExtraInvalidatedValues(ValueList &Values,
+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 &Values) const {
+void BlockCall::getExtraInvalidatedValues(ValueList &Values,
+  RegionAndSymbolInval

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;
  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 = &x;
+  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 = &x;
+  t.foo();
+  clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatConstMethodDoesInvalidateContainedPointedAtMemory() {
+  int x = 1;
+  Outer t;
+  t.x = 2;
+  t.in.p = &x;
+  t.foo();
+  clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(t.x == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(t.in.p == &x); // 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, &ETraits);
 
   // 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 &Values) const {
-  Values.push_back(getCXXThisVal());
+void CXXInstanceCall::getExtraInvalidatedValues(ValueList &Values,
+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 &Values) const {
+void BlockCall::getExtraInvalidatedValues(ValueList &Values,
+  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 &Values) const {
+void CXXConstructorCall::getExtraInvalidatedValues(ValueList &Values,
+   RegionAndSymbolInvalidationTraits *ETraits) const {
   if (Data)
 Values.

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

r249385 - [VFS] Transition clang-format to use an in-memory FS.

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Tue Oct  6 05:04:08 2015
New Revision: 249385

URL: http://llvm.org/viewvc/llvm-project?rev=249385&view=rev
Log:
[VFS] Transition clang-format to use an in-memory FS.

Apart from being cleaner this also means that clang-format no longer has
access to the host file system. This isn't necessary because clang-format
never reads includes :)

Includes minor tweaks and bugfixes found in the VFS implementation while
running clang-format tests.

Modified:
cfe/trunk/include/clang/Basic/VirtualFileSystem.h
cfe/trunk/lib/Basic/VirtualFileSystem.cpp
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/tools/clang-format/ClangFormat.cpp
cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp

Modified: cfe/trunk/include/clang/Basic/VirtualFileSystem.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/VirtualFileSystem.h?rev=249385&r1=249384&r2=249385&view=diff
==
--- cfe/trunk/include/clang/Basic/VirtualFileSystem.h (original)
+++ cfe/trunk/include/clang/Basic/VirtualFileSystem.h Tue Oct  6 05:04:08 2015
@@ -274,8 +274,12 @@ class InMemoryFileSystem : public FileSy
 public:
   InMemoryFileSystem();
   ~InMemoryFileSystem() override;
+  /// Add a buffer to the VFS with a path. The VFS owns the buffer.
   void addFile(const Twine &Path, time_t ModificationTime,
std::unique_ptr Buffer);
+  /// Add a buffer to the VFS with a path. The VFS does not own the buffer.
+  void addFileNoOwn(const Twine &Path, time_t ModificationTime,
+llvm::MemoryBuffer *Buffer);
   StringRef toString() const;
 
   llvm::ErrorOr status(const Twine &Path) override;

Modified: cfe/trunk/lib/Basic/VirtualFileSystem.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/VirtualFileSystem.cpp?rev=249385&r1=249384&r2=249385&view=diff
==
--- cfe/trunk/lib/Basic/VirtualFileSystem.cpp (original)
+++ cfe/trunk/lib/Basic/VirtualFileSystem.cpp Tue Oct  6 05:04:08 2015
@@ -528,6 +528,13 @@ void InMemoryFileSystem::addFile(const T
   }
 }
 
+void InMemoryFileSystem::addFileNoOwn(const Twine &P, time_t ModificationTime,
+  llvm::MemoryBuffer *Buffer) {
+  return addFile(P, ModificationTime,
+ llvm::MemoryBuffer::getMemBuffer(
+ Buffer->getBuffer(), Buffer->getBufferIdentifier()));
+}
+
 static ErrorOr
 lookupInMemoryNode(const InMemoryFileSystem &FS, detail::InMemoryDirectory 
*Dir,
const Twine &P) {
@@ -541,6 +548,15 @@ lookupInMemoryNode(const InMemoryFileSys
 
   auto I = llvm::sys::path::begin(Path), E = llvm::sys::path::end(Path);
   while (true) {
+// Skip over ".".
+// FIXME: Also handle "..".
+if (*I == ".") {
+  ++I;
+  if (I == E)
+return Dir;
+  continue;
+}
+
 detail::InMemoryNode *Node = Dir->getChild(*I);
 ++I;
 if (!Node)

Modified: cfe/trunk/lib/Format/Format.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=249385&r1=249384&r2=249385&view=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Tue Oct  6 05:04:08 2015
@@ -1792,18 +1792,17 @@ tooling::Replacements reformat(const For
   if (Style.DisableFormat)
 return tooling::Replacements();
 
-  FileManager Files((FileSystemOptions()));
+  IntrusiveRefCntPtr InMemoryFileSystem(
+  new vfs::InMemoryFileSystem);
+  FileManager Files(FileSystemOptions(), InMemoryFileSystem);
   DiagnosticsEngine Diagnostics(
   IntrusiveRefCntPtr(new DiagnosticIDs),
   new DiagnosticOptions);
   SourceManager SourceMgr(Diagnostics, Files);
-  std::unique_ptr Buf =
-  llvm::MemoryBuffer::getMemBuffer(Code, FileName);
-  const clang::FileEntry *Entry =
-  Files.getVirtualFile(FileName, Buf->getBufferSize(), 0);
-  SourceMgr.overrideFileContents(Entry, std::move(Buf));
-  FileID ID =
-  SourceMgr.createFileID(Entry, SourceLocation(), clang::SrcMgr::C_User);
+  InMemoryFileSystem->addFile(FileName, 0,
+  llvm::MemoryBuffer::getMemBuffer(Code, 
FileName));
+  FileID ID = SourceMgr.createFileID(Files.getFile(FileName), SourceLocation(),
+ clang::SrcMgr::C_User);
   SourceLocation StartOfFile = SourceMgr.getLocForStartOfFile(ID);
   std::vector CharRanges;
   for (const tooling::Range &Range : Ranges) {

Modified: cfe/trunk/tools/clang-format/ClangFormat.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-format/ClangFormat.cpp?rev=249385&r1=249384&r2=249385&view=diff
==
--- cfe/trunk/tools/clang-format/ClangFormat.cpp (original)
+++ cfe/trunk/tools/clang-format/ClangFormat.cpp Tue Oct  6 05:04:08 2015
@@ -109,11 +109,11 @

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&view=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&r1=249387&r2=249388&view=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 &Replaces) 
{
-  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&view=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&r1=249388&r2=249389&view=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

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

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&view=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&r1=249390&r2=249391&view=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 &CommandLine,
+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&r1=249390&r2=249391&view=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 &CommandLine,
+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&r1=249390&r2=249391&view=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&r1=249390&r2=249391&view=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 &Target : llvm::TargetRegistry::targets()) {
+std::string Error;
+if (llvm::TargetRegistry::lookupTarget(Target.getName(), Error) ==
+&Target) {
+  return Target.getName();
+}

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&view=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&r1=249391&r2=249392&view=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&r1=249391&r2=249392&view=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: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=249392&r1=249391&r2=249392&view=diff
=

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&view=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&r1=249393&r2=249394&view=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&r1=249393&r2=249394&view=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

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&view=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&r1=249394&r2=249395&view=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

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

[Piotr Zegar via cfe-commits]

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.

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.

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

[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 &Result) 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 &Result) 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 &Result) 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 &Diag)
+  : 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 &Diag;
+};
+

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

[Manuel Klimek via cfe-commits]

klimek added inline comments.


Comment at: unittests/clang-tidy/OverlappingReplacementsTest.cpp:133
@@ +132,3 @@
+
+TEST(OverlappingReplacementsTest, TestingChecksWorkAsExpected) {
+  const char Code[] =

I'd split it up and give the tests better names. TestingChecksWorksAsExpected 
doesn't really give new information :)


http://reviews.llvm.org/D13469



___
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.

[Angel Garcia via cfe-commits]

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

This test was intended to ensure that we don't have a bug in the mock checks. 
I've split it up into one test for each mock check, I hope it is more 
descriptive now.


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,375 @@
+//=== 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 &Result) 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 &Result) 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 &Result) 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 &Diag)
+  : 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:
+

[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&view=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&r1=249398&r2=249399&view=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&r1=249398&r2=249399&view=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&r1=249398&r2=249399&view=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&view=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&view=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 
---

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

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

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 &Result) 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 &Result) 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 &Result) 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 &Diag)
+  : VD(VD), NewName(NewName), Diag(Diag) {}
+  bool VisitDeclRefExpr(DeclRefExpr *E) {
+if (const ValueDecl *D = E->getDecl()) {
+  if (VD->getCanonicalDecl() == D->getCanonicalDecl()) {
+Diag << FixItHint::CreateReplacement(
+CharSourc

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

[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&view=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&r1=249401&r2=249402&view=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&r1=249401&r2=249402&view=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 &Check, ast_matchers::MatchFinder &Finder,
+  TestClangTidyAction(SmallVectorImpl> &Checks,
+  ast_matchers::MatchFinder &Finder,
   ClangTidyContext &Context)
-  : Check(Check), Finder(Finder), Context(Context) {}
+  : Checks(Checks), Finder(Finder), Context(Context) {}
 
 private:
   std::unique_ptr CreateASTConsumer(CompilerInstance &Compiler,
@@ -36,17 +38,37 @@ private:
 Context.setCurrentFile(File);
 Context.setASTContext(&Compiler.getASTContext());
 
-Check.registerMatchers(&Finder);
-Check.registerPPCallbacks(Compiler);
+for (auto &Check : Checks) {
+  Check->registerMatchers(&Finder);
+  Check->registerPPCallbacks(Compiler);
+}
 return Finder.newASTConsumer();
   }
 
-  ClangTidyCheck &Check;
+  SmallVectorImpl> &Checks;
   ast_matchers::MatchFinder &Finder;
   ClangTidyContext &Context;
 };
 
-template 
+template  struct CheckFactory {
+  static void
+  createChecks(ClangTidyContext *Context,
+   SmallVectorImpl> &Result) {
+CheckFactory::createChecks(Context, Result);
+CheckFactory::createChecks(Context, Result);
+  }
+};
+
+template  struct CheckFactory {
+  static void
+  createChecks(ClangTidyContext *Context,
+   SmallVectorImpl> &Result) {
+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 &Filename = "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", &Context);
 
   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, 1> Checks;
+  CheckFactory::createChecks(&Context, 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 &FileContent : 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&view=auto
==
--- 
clang-tools-extra/trunk/unittests/clang-tidy/OverlappingReplacement

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

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

On Mon, Oct 5, 2015 at 6:14 PM, Matthias Gehre  wrote:

> mgehre marked an inline comment as done.

> 

> 

>  Comment at: 
> clang-tidy/cppcoreguidelines/ProTypeStaticCastDowncastCheck.cpp:53

>  @@ +52,3 @@

>  +  } else {

>  +diag(MatchedCast->getOperatorLoc(), "do not use static_cast to cast 
> from base class to derived class.");

>  +  }

> 

>  

> 

> aaron.ballman wrote:

> 

> > What is the alternative the user should use in this instance?

> 

> 

> The alternative is to either

> 

> 1. change the program logic to not require and downcast of a non-polymorphic 
> type or

> 2. add NOLINT to notify your coworkers that this may be the spot where 
> something goes wrong

> 

>   In the end, these rules together should eliminate some classes of undefined 
> behavior and crashes. If the application still crashes, you should just need 
> to look at the (hopefully few) places of NOLINT.

> 

>   Anyway, I didn't make the rules. See Herb Sutters answer here: 
> https://github.com/isocpp/CppCoreGuidelines/issues/270


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.

> 

>  Comment at: 
> clang-tidy/cppcoreguidelines/ProTypeStaticCastDowncastCheck.cpp:33

>  @@ +32,3 @@

>  +  const auto *SourceDecl = SourceType->getPointeeCXXRecordDecl();

>  +  if(!SourceDecl)

>  +SourceDecl = SourceType->getAsCXXRecordDecl();

> 

>  

> 

> aaron.ballman wrote:

> 

> > In the event it's not a pointer or a reference, why are you getting the 
> > source as a value type?

> 

> 

> Source type could be no pointer nor ref like in:

> 

>   Base B0;

>   auto R0 = static_cast(B0);


Ah, interesting! I was guessing that case would have had an lvalue reference 
type for B0, but I'm guessing it's already gone through lvalue to rvalue 
conversion before hitting the cast node (or my intuition is just off).



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());

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).


http://reviews.llvm.org/D13368



___
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&view=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&r1=249402&r2=249403&view=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 &Target : llvm::TargetRegistry::targets()) {
 std::string Error;
-if (llvm::TargetRegistry::lookupTarget(Target.getName(), Error) ==
-&Target) {
-  return Target.getName();
+StringRef TargetName(Target.getName());
+if (TargetName == "x86-64")
+  TargetName = "x86_64";
+if (llvm::TargetRegistry::lookupTarget(TargetName, Error) == &Target) {
+  return TargetName;
 }
   }
   return "";


___
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]

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

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] 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

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] 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

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&view=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&r1=249409&r2=249410&view=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&r1=249409&r2=249410&view=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 = &Clang->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&r1=249409&r2=249410&view=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(&Invocation->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-comm

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&view=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&r1=249408&r2=249409&view=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&r1=249408&r2=249409&view=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&view=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&r1=249407&r2=249408&view=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

[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(
+  Code.toNullTerminatedStr

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

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Tue Oct  6 10:04:13 2015
New Revision: 249413

URL: http://llvm.org/viewvc/llvm-project?rev=249413&view=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&r1=249412&r2=249413&view=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), &Action,
-nullptr, PCHContainerOps);
+Files.get(), PCHContainerOps);
 
   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

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&view=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&r1=249394&r2=249395&view=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] 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] 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 &FilenameWithContent : VirtualMappedFiles) {
-Invocation.mapVirtualFile(F

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 &Result) {

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 &Value);
+

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] 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] 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

[clang-tools-extra] r249429 - Improved the misc-move-constructor-init check to identify arguments that are passed by value but copy assigned to class data members when the non-deleted move constructor

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Tue Oct  6 11:27:03 2015
New Revision: 249429

URL: http://llvm.org/viewvc/llvm-project?rev=249429&view=rev
Log:
Improved the misc-move-constructor-init check to identify arguments that are 
passed by value but copy assigned to class data members when the non-deleted 
move constructor is a better fit.

Patch by Felix Berger!

Added:
clang-tools-extra/trunk/clang-tidy/utils/Matchers.h
clang-tools-extra/trunk/clang-tidy/utils/TypeTraits.cpp
clang-tools-extra/trunk/clang-tidy/utils/TypeTraits.h
Modified:
clang-tools-extra/trunk/clang-tidy/misc/MoveConstructorInitCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/MoveConstructorInitCheck.h
clang-tools-extra/trunk/clang-tidy/modernize/PassByValueCheck.cpp
clang-tools-extra/trunk/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp
clang-tools-extra/trunk/clang-tidy/utils/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/utils/IncludeSorter.cpp
clang-tools-extra/trunk/clang-tidy/utils/IncludeSorter.h

clang-tools-extra/trunk/docs/clang-tidy/checks/misc-move-constructor-init.rst
clang-tools-extra/trunk/test/clang-tidy/misc-move-constructor-init.cpp

Modified: clang-tools-extra/trunk/clang-tidy/misc/MoveConstructorInitCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/MoveConstructorInitCheck.cpp?rev=249429&r1=249428&r2=249429&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/misc/MoveConstructorInitCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/misc/MoveConstructorInitCheck.cpp Tue 
Oct  6 11:27:03 2015
@@ -8,14 +8,42 @@
 
//===--===//
 
 #include "MoveConstructorInitCheck.h"
+#include "../utils/Matchers.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Lex/Preprocessor.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 
+namespace {
+
+unsigned int
+parmVarDeclRefExprOccurences(const ParmVarDecl &MovableParam,
+ const CXXConstructorDecl &ConstructorDecl,
+ ASTContext &Context) {
+  unsigned int Occurrences = 0;
+  auto AllDeclRefs =
+  findAll(declRefExpr(to(parmVarDecl(equalsNode(&MovableParam);
+  Occurrences += match(AllDeclRefs, *ConstructorDecl.getBody(), 
Context).size();
+  for (const auto *Initializer : ConstructorDecl.inits()) {
+Occurrences += match(AllDeclRefs, *Initializer->getInit(), Context).size();
+  }
+  return Occurrences;
+}
+
+} // namespace
+
+MoveConstructorInitCheck::MoveConstructorInitCheck(StringRef Name,
+   ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  IncludeStyle(IncludeSorter::parseIncludeStyle(
+  Options.get("IncludeStyle", "llvm"))) {}
+
 void MoveConstructorInitCheck::registerMatchers(MatchFinder *Finder) {
   // Only register the matchers for C++11; the functionality currently does not
   // provide any benefit to other languages, despite being benign.
@@ -31,13 +59,65 @@ void MoveConstructorInitCheck::registerM
 withInitializer(cxxConstructExpr(hasDeclaration(
 cxxConstructorDecl(isCopyConstructor())
 .bind("ctor")
-.bind("init",
+.bind("move-init",
+  this);
+
+  auto NonConstValueMovableAndExpensiveToCopy =
+  qualType(allOf(unless(pointerType()), unless(isConstQualified()),
+ hasDeclaration(cxxRecordDecl(hasMethod(cxxConstructorDecl(
+ isMoveConstructor(), unless(isDeleted()),
+ matchers::isExpensiveToCopy()));
+  Finder->addMatcher(
+  cxxConstructorDecl(
+  allOf(
+  unless(isMoveConstructor()),
+  hasAnyConstructorInitializer(withInitializer(cxxConstructExpr(
+  hasDeclaration(cxxConstructorDecl(isCopyConstructor())),
+  hasArgument(
+  0, declRefExpr(
+ to(parmVarDecl(
+hasType(
+
NonConstValueMovableAndExpensiveToCopy))
+.bind("movable-param")))
+ .bind("init-arg")))
+  .bind("ctor-decl"),
   this);
 }
 
 void MoveConstructorInitCheck::check(const MatchFinder::MatchResult &Result) {
+  if (Result.Nodes.getNodeAs("move-init") != nullptr)
+handleMoveConstructor(Result);
+  if (Result.Nodes.getNodeAs("movable-param") != nullptr)
+handleParamNotMoved(Result);
+}
+
+void MoveConstructorInitCheck::handleParamNotMoved(
+const MatchFinder::MatchResult &Result) {
+  c

[clang-tools-extra] r249430 - Attempting to appease the CMake build bots after r249429.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Tue Oct  6 11:32:50 2015
New Revision: 249430

URL: http://llvm.org/viewvc/llvm-project?rev=249430&view=rev
Log:
Attempting to appease the CMake build bots after r249429.

Modified:
clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt

Modified: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt?rev=249430&r1=249429&r2=249430&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt Tue Oct  6 11:32:50 
2015
@@ -29,4 +29,5 @@ add_clang_library(clangTidyMiscModule
   clangBasic
   clangLex
   clangTidy
+  clangTidyUtils
   )


___
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

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#260821, @evgeny777 wrote:

> 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).


Hah, good point, the test case is in lldb, not in clang, for that very reason. 
:-P Carry on (I must need more sleep).


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.

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] D10305: [Clang Static Analyzer] Bug identification

[Anna Zaks via cfe-commits]

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

Thanks for working on this!

LGTM, Anna.


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] D13444: [clang-tidy] Clocky module and multiple check from my repository

[Piotr Zegar via cfe-commits]

Splinting a patch into separate check it's not a problem. I have organized
commits in that way.

About a 'author' in name, organization is also a author, so clocky is a
'one man organization' :D.

It's much more simple, I have no benefits in 'anonymous' contribution, in
that case I would rather go into separate project.
>From a user-point of view, i'm sure that no-one would like to see 'clocky',
'google', 'llvm' in a compiler output, and I agree with that.
But on file level it's much more easier to organize checks in that way, as
I't keep author more responsible for own checkers.

For future my proposal could be:
- leave files/folders name as they are
- add a check 'tag' engine:

So output from --list-checks-ex could be:
Module Check-nameTags
google  explicit-constructor
 [reliability, security]
clocky   inefficient-container-insert  [performance,
c++11]

In this way, user would be able to run check in few ways:
clocky-inefficient-container-insert,
performance-inefficient-container-insert, c++11-inefficient-container-insert,
c++11-performance-inefficient-container-insert, inefficient-container-insert,
in short any combination of tags would be accepted. But it would select a
check that have all tags.
And a name that user used to enable check would show up in warning, a
default would be a first tag name performance-inefficient-container-insert,
if not available then module name.
Also user would be able to enable all check from specific tag like
--checks= "-*, performance"., or pair of tags --checks="performance-c++11"
to enable all checks that are c++11 performance checks.
A output from --list-checks would be a list of default names.

In that way no new 'modules' would need to be created to tag a checks (
cert-*).
Tests still would use a module name as prefix and in file names.
Module could be also printed as a tag, instead of separate column.
Checks names would need to be unique.

Right now documentation of clang-tidy checks is poor, some template should
be used.
Now I see that some of checks are duplicated in both google and misc
module, that also would need to be resolved out.

Thats, just my ideas...

2015-10-06 17:50 GMT+02:00 Aaron Ballman :

> 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
> >
> >
> >
>



-- 
inż. Piotr Zegar
m...@piotrzegar.pl
http://www.piotrzegar.pl
___
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

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&view=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&r1=249436&r2=249437&view=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&r1=249436&r2=249437&view=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&r1=249436&r2=249437&view=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

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 &TC,
>> - const ArgList &Args) {
>> -  const llvm::Triple &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 &TC) {
>> -  // The runtimes are located in the OS-specific resource directory.
>> -  SmallString<128> Res(TC.getDriver().ResourceDir);
>> -  const llvm::Triple &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 &TC, const ArgList 
>> &Args,
>> -  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 &TC,
>> -  const llvm::opt::ArgList &Args,
>> -  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 &TC, const ArgList &Args,
>> ArgStringList &CmdArgs) {
>> -  CmdArgs.push_back(getCompilerRTArgString(TC, Args, "builtins"));
>> -}
>> -
>> -static void addProfileRT(const ToolChain &TC, const ArgList &Args,
>> - ArgStringList &CmdArgs) {
>> -  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)
>>  CmdA

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&view=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&r1=249439&r2=249440&view=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&r1=249439&r2=249440&view=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&r1=249439&r2=249440&view=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&view=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&r1=249443&r2=249444&view=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

[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&view=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&r1=249444&r2=249445&view=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

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&view=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&r1=249412&r2=249413&view=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),
> &Action,
> -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

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&view=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

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

[PATCH] D13482: Revised Initial patch for PS4 toolchain

[Katya Romanova via cfe-commits]

kromanova created this revision.
kromanova added reviewers: filcab, echristo, alexr, probinson.
kromanova added subscribers: silvas, cfe-commits, chapuni, jroelofs, asl, 
pgousseau, gbedwell.
kromanova set the repository for this revision to rL LLVM.

Initial patch for PS4 toolchain was created here: http://reviews.llvm.org/D11279
When this patch was committed in r248546, it caused 2 distinct problems.

(1) Many failures were reported on recently set up PS4 bot. The PS4 driver 
intentionally reported warnings if PS4 SDK was missing. The new buildbot didn't 
have PS4 SDK directory installed. This "combination" caused failures for all 
the tests that had -Werror option.
(2) ps4-linker-win.c failed on all the Windows bots.

This commit was reverted in r248548 to make the bots green again and to decide 
on how to deal with missing PS4 SDK directory issue.

The new patch should take care of both problems mentioned above.
Since review for http://reviews.llvm.org/D11279 was owned by Filipe Cabecinhas 
and already closed, it's probably easier for me to open a new code review. Feel 
free to compare this new patch to the latest patch in D11279.
Here is the summary of the changes:

1.  include/clang/Basic/DiagnosticDriverKinds.td 
Added DefaultIgnore attribute to InvalidOrNonExistentDirectory. 
By default, the PS4 driver won't report a warning about missing PS4 SDK. This 
behavior could be changed if  -Weverything or 
-Winvalid-or-nonexistent-directory options are passed.

2.  Changed 
test/Driver/ps4-linker-non-win.c to use -fuse-ld=gold instead of -linker=gold
test/Driver/ps4-linker-win.c to use -fuse-ld=gold instead of -linker=gold
We do not support "-linker" option anymore. Use "-fuse-ld" instead.

3. test/Driver/ps4-sdk-root.c
Added -Winvalid-or-nonexistent-directory to all the RUN lines to force the 
warning (this is the only test where we want to have the warnings about missing 
PS4 SDK directory enabled).

4.  test/Driver/rtti-options.cpp 
Removed all the changes. No changes are needed, since the warning is not 
reported by default anymore.

Origianal patch extracted by Filipe Cabecinhas, me (Katya Romanova), and Pierre 
Gousseau.






Repository:
  rL LLVM

http://reviews.llvm.org/D13482

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Basic/DiagnosticGroups.td
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains.cpp
  lib/Driver/ToolChains.h
  lib/Driver/Tools.cpp
  lib/Driver/Tools.h
  lib/Frontend/InitHeaderSearch.cpp
  test/Driver/Inputs/scei-ps4_tree/target/include/.keep
  test/Driver/Inputs/scei-ps4_tree/target/include_common/.keep
  test/Driver/debug-options.c
  test/Driver/ps4-header-search.c
  test/Driver/ps4-linker-non-win.c
  test/Driver/ps4-linker-win.c
  test/Driver/ps4-pic.c
  test/Driver/ps4-sdk-root.c
  test/Driver/stack-protector.c

Index: test/Driver/stack-protector.c
===
--- test/Driver/stack-protector.c
+++ test/Driver/stack-protector.c
@@ -23,3 +23,12 @@
 // RUN: %clang -fstack-protector-all -### %s 2>&1 | FileCheck %s -check-prefix=SSP-ALL
 // SSP-ALL: "-stack-protector" "3"
 // SSP-ALL-NOT: "-stack-protector-buffer-size" 
+
+// RUN: %clang -target x86_64-scei-ps4 -### %s 2>&1 | FileCheck %s -check-prefix=SSP-PS4
+// RUN: %clang -target x86_64-scei-ps4 -fstack-protector -### %s 2>&1 | FileCheck %s -check-prefix=SSP-PS4
+// SSP-PS4: "-stack-protector" "2"
+// SSP-PS4-NOT: "-stack-protector-buffer-size"
+
+// RUN: %clang -target x86_64-scei-ps4 -fstack-protector --param ssp-buffer-size=16 -### %s 2>&1 | FileCheck %s -check-prefix=SSP-PS4-BUF
+// SSP-PS4-BUF: "-stack-protector" "2"
+// SSP-PS4-BUF: "-stack-protector-buffer-size" "16"
Index: test/Driver/ps4-sdk-root.c
===
--- test/Driver/ps4-sdk-root.c
+++ test/Driver/ps4-sdk-root.c
@@ -0,0 +1,48 @@
+// REQUIRES: x86-registered-target
+
+// Check that ps4-clang doesn't report a warning message when locating
+// system header files (either by looking at the value of SCE_PS4_SDK_DIR
+// or relative to the location of the compiler driver), if "-nostdinc",
+// "--sysroot" or "-isysroot" option is specified on the command line.
+// Otherwise, check that ps4-clang reports a warning.
+
+// Check that clang doesn't report a warning message when locating
+// system libraries (either by looking at the value of SCE_PS4_SDK_DIR
+// or relative to the location of the compiler driver), if "-c", "-S", "-E",
+// "--sysroot", "-nostdlib" or "-nodefaultlibs" option is specified on
+// the command line.
+// Otherwise, check that ps4-clang reports a warning.
+
+// setting up SCE_PS4_SDK_DIR to existing location, which is not a PS4 SDK.
+// RUN: env SCE_PS4_SDK_DIR=.. %clang -Winvalid-or-nonexistent-directory -### -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=WARN-SYS-HEADERS -check-prefix=WARN-SYS-LIBS -check-prefix=NO-WARN %s
+
+// RUN: env SCE_PS4_SDK_DIR=.. %clang -Winvalid-or-nonexiste

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

[Evgeniy Stepanov via cfe-commits]

eugenis 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

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.



http://reviews.llvm.org/D13407



___
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&view=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&r1=249457&r2=249458&view=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

[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&view=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&r1=249460&r2=249461&view=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: [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 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
cppcoreguidelines-pro-type-reinterpret-cast
+   cppcoreguidelines-pro-type-static-cast-downcast
google-build-explicit-make-pair
google-build-namespaces
google-build-using-namespace
Index: docs/clang-tidy/checks/cppcoreguidelines-pro-type-static-cast-downcast.rst

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

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 &Result) 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 &Result) {
+  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] 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* __stream) {retur

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

[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&view=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&r1=249474&r2=249475&view=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&r1=249474&r2=249475&view=diff
==
--

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

cfe-commits@lists.llvm.org

[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]

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') == 'a');
+assert(std::toupper('a') == 'A');
 }
___
cfe-commits mailing list
cfe-commits@lists.ll

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 ENOTEMPTY 9953
-#endif
-
-#ifndef ENOEXEC
-#define ENOEXEC 9954
-#endif
-
-#ifndef EEXIST
-#define EEXIS

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&view=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] D12747: Implement [depr.c.headers]

[Richard Smith via cfe-commits]

, 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/csetjmp include/csetjmp
index d0b2c07..58a9c73 100644
--- include/csetjmp
+++ include/csetjmp
@@ -38,10 +38,6 @@ void longjmp(jmp_buf env, int val);
 #pragma GCC system_header
 #endif
 
-#ifndef setjmp
-#define setjmp(env) setjmp(env)
-#endif
-
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 using ::jmp_buf;
diff --git include/setjmp.h include/setjmp.h
new file mode 100644
index 000..d429c86
--- /dev/null
+++ include/setjmp.h
@@ -0,0 +1,40 @@
+// -*- C++ -*-
+//===--- setjmp.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_SETJMP_H
+#define _LIBCPP_SETJMP_H
+
+/*
+setjmp.h synopsis
+
+Macros:
+
+setjmp
+
+Types:
+
+jmp_buf
+
+void longjmp(jmp_buf env, int val);
+
+*/
+
+#include <__config>
+#include_next 
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#pragma GCC system_header
+#endif
+
+#ifndef setjmp
+#define setjmp(env) setjmp(env)
+#endif
+
+#endif  // _LIBCPP_SETJMP_H
diff --git test/std/depr/depr.c.headers/setjmp_h.pass.cpp 
test/std/depr/depr.c.headers/setjmp_h.pass.cpp
index 36f4253..9bc35b7 100644
--- test/std/depr/depr.c.headers/setjmp_h.pass.cpp
+++ test/std/depr/depr.c.headers/setjmp_h.pass.cpp
@@ -12,6 +12,10 @@
 #include 
 #include 
 
+#ifndef setjmp
+#error setjmp not defined
+#endif
+
 int main()
 {
 jmp_buf jb;
___
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 _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR bool operator>(nullptr_t, 
nullptr_t) {return false;}
-frie

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&view=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&r1=249394&r2=249395&view=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*)");
 static_assert(std::is_unsigned::value,
@@ -34,4 +38,22 @@ int main()
   "std::is_signed::value");
 static_assert(std::is_integral::value,
 

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

[Sean Silva via cfe-commits]

+extern "C++" {
+#include <__nullptr>
+using std::nullptr_t;
+}

Does this even compile with modules?


-- 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 
>>> 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] D12747: Implement [depr.c.headers]

[Richard Smith via cfe-commits]

: like , this provides only pieces if __need_FILE or
__need___FILE is defined
: likewise, but for __need_malloc_and_calloc
 and  are straightforward
 is tricky: C libraries that don't implement the right interface
provide non-const-correct functions like "wchar_t *wmemchr(const wchar_t*,
wchar_t, size_t)". We can't fix that, so we don't try.

On Tue, Oct 6, 2015 at 3:57 PM, Richard Smith  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 
>>> 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/cstdio include/cstdio
index 61985d6..50fdd34 100644
--- include/cstdio
+++ include/cstdio
@@ -103,16 +103,6 @@ void perror(const char* s);
 #pragma GCC system_header
 #endif
 
-// snprintf
-#if defined(_LIBCPP_MSVCRT)
-#include "support/win32/support.h"
-#endif
-
-#undef getc
-#undef putc
-#undef clearerr
-#undef feof
-#undef ferror
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 using ::FILE;
diff --git include/stdio.h include/stdio.h
new file mode 100644
index 000..bebe1fb
--- /dev/null
+++ include/stdio.h
@@ -0,0 +1,121 @@
+// -*- C++ -*-
+//=== stdio.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_FILE) || defined(__need___FILE)
+#include_next 
+
+#elif !defined(_LIBCPP_STDIO_H)
+#define _LIBCPP_STDIO_H
+
+/*
+stdio.h synopsis
+
+Macros:
+
+BUFSIZ
+EOF
+FILENAME_MAX
+FOPEN_MAX
+L_tmpnam
+NULL
+SEEK_CUR
+SEEK_END
+SEEK_SET
+TMP_MAX
+_IOFBF
+_IOLBF
+_IONBF
+stderr
+stdin
+stdout
+
+Types:
+
+FILE
+fpos_t
+size_t
+
+int remove(const char* filename);
+int rename(const char* old, const char* new);
+FILE* tmpfile(void);
+char* tmpnam(char* s);
+int fclose(FILE* stream);
+int fflush(FILE* stream);
+FILE* fopen(const char* restrict filename, const char* restrict mode);
+FILE* freopen(const char* restrict filename, const char * restrict mode,
+  FILE * restrict stream);
+void setbuf(FILE* restrict stream, char* restrict buf);
+int setvbuf(FILE* restrict stream, char* restrict buf, int mode, size_t size);
+int fprintf(FIL

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

[Richard Smith via cfe-commits]

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.


> 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

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&view=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&r1=249483&r2=249484&view=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) {
   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, &Ctx);
 }
 
-
 /// \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