date:20171109

[PATCH] D33440: clang-format: better handle statement macros

2017-11-09 Thread Francois Ferrand via Phabricator via cfe-commits

Typz added a comment.

ping?


https://reviews.llvm.org/D33440



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

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

2017-11-09 Thread Francois Ferrand via Phabricator via cfe-commits

Typz added a comment.

ping?


https://reviews.llvm.org/D32478



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

Re: [PATCH] D5767: Template Instantiation Observer + a few other templight-related changes

2017-11-09 Thread Manuel Klimek via cfe-commits

On Thu, Nov 9, 2017 at 4:02 AM Richard Smith - zygoloid via Phabricator <
revi...@reviews.llvm.org> wrote:

> rsmith added a comment.
>
> I'm not entirely sure what's happening with this and
> https://reviews.llvm.org/D38818, but the direction looks good to me, and
> I left a couple of comments on the other review thread.
>

This patch had some of my comments addressed :(
(around how the interface is structured)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

2017-11-09 Thread Sam McCall via Phabricator via cfe-commits

sammccall updated this revision to Diff 122206.
sammccall added a comment.

Add test verifying that working directory is compile_flags's parent.


https://reviews.llvm.org/D39799

Files:
  docs/JSONCompilationDatabase.rst
  include/clang/Tooling/CompilationDatabase.h
  lib/Tooling/CompilationDatabase.cpp
  test/Tooling/Inputs/fixed-header.h
  test/Tooling/fixed-database.cpp

Index: test/Tooling/fixed-database.cpp
===
--- /dev/null
+++ test/Tooling/fixed-database.cpp
@@ -0,0 +1,18 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/Src
+// RUN: cp "%s" "%t/Src/test.cpp"
+// RUN: mkdir -p %t/Include
+// RUN: cp "%S/Inputs/fixed-header.h" "%t/Include/"
+// -I flag is relative to %t (where compile_flags is), not Src/.
+// RUN: echo '-IInclude/' >> %t/compile_flags.txt
+// RUN: echo "-Dklazz=class" >> %t/compile_flags.txt
+// RUN: echo '-std=c++11' >> %t/compile_flags.txt
+// RUN: clang-check "%t/Src/test.cpp" 2>&1
+// RUN: not clang-check "%s" 2>&1 | FileCheck "%s" -check-prefix=NODB
+
+// NODB: unknown type name 'klazz'
+klazz F{};
+
+// NODB: 'fixed-header.h' file not found
+#include "fixed-header.h"
+static_assert(SECRET_SYMBOL == 1, "");
Index: test/Tooling/Inputs/fixed-header.h
===
--- /dev/null
+++ test/Tooling/Inputs/fixed-header.h
@@ -0,0 +1 @@
+#define SECRET_SYMBOL 1
Index: lib/Tooling/CompilationDatabase.cpp
===
--- lib/Tooling/CompilationDatabase.cpp
+++ lib/Tooling/CompilationDatabase.cpp
@@ -26,6 +26,7 @@
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Support/Host.h"
+#include "llvm/Support/LineIterator.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
@@ -302,8 +303,22 @@
   std::vector StrippedArgs;
   if (!stripPositionalArgs(CommandLine, StrippedArgs, ErrorMsg))
 return nullptr;
-  return std::unique_ptr(
-  new FixedCompilationDatabase(Directory, StrippedArgs));
+  return llvm::make_unique(Directory, StrippedArgs);
+}
+
+std::unique_ptr
+FixedCompilationDatabase::loadFromFile(StringRef Path, std::string ) {
+  ErrorMsg.clear();
+  llvm::ErrorOr File =
+  llvm::MemoryBuffer::getFile(Path);
+  if (std::error_code Result = File.getError()) {
+ErrorMsg = "Error while opening fixed database: " + Result.message();
+return nullptr;
+  }
+  std::vector Args{llvm::line_iterator(**File),
+llvm::line_iterator()};
+  return llvm::make_unique(
+  llvm::sys::path::parent_path(Path), std::move(Args));
 }
 
 FixedCompilationDatabase::
@@ -334,6 +349,22 @@
   return std::vector();
 }
 
+namespace {
+
+class FixedCompilationDatabasePlugin : public CompilationDatabasePlugin {
+  std::unique_ptr
+  loadFromDirectory(StringRef Directory, std::string ) override {
+SmallString<1024> DatabasePath(Directory);
+llvm::sys::path::append(DatabasePath, "compile_flags.txt");
+return FixedCompilationDatabase::loadFromFile(DatabasePath, ErrorMessage);
+  }
+};
+
+static CompilationDatabasePluginRegistry::Add
+X("fixed-compilation-database", "Reads plain-text flags file");
+
+} // namespace
+
 namespace clang {
 namespace tooling {
 
Index: include/clang/Tooling/CompilationDatabase.h
===
--- include/clang/Tooling/CompilationDatabase.h
+++ include/clang/Tooling/CompilationDatabase.h
@@ -182,6 +182,11 @@
   int , const char *const *Argv, std::string ,
   Twine Directory = ".");
 
+  /// Reads flags from the given file, one-per line.
+  /// Returns nullptr and sets ErrorMessage if we can't read the file.
+  static std::unique_ptr
+  loadFromFile(StringRef Path, std::string );
+
   /// \brief Constructs a compilation data base from a specified directory
   /// and command line.
   FixedCompilationDatabase(Twine Directory, ArrayRef CommandLine);
Index: docs/JSONCompilationDatabase.rst
===
--- docs/JSONCompilationDatabase.rst
+++ docs/JSONCompilationDatabase.rst
@@ -91,3 +91,9 @@
 the top of the build directory. Clang tools are pointed to the top of
 the build directory to detect the file and use the compilation database
 to parse C++ code in the source tree.
+
+Alternatives
+
+For simple projects, Clang tools also recognize a compile_flags.txt file.
+This should contain one flag per line. The same flags will be used to compile
+any file.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33440: clang-format: better handle statement macros

2017-11-09 Thread Daniel Jasper via Phabricator via cfe-commits

djasper added a comment.

Out of curiosity, will this be able to fix the two situations that you get for 
python extension?
There, you usually have a PyObject_HEAD with out semicolon in a struct and than 
a PyObject_HEAD_INIT(..) in a braced init list. More info:
http://starship.python.net/crew/mwh/toext/node20.html

Comment at: lib/Format/FormatTokenLexer.cpp:642
   tok::pp_define) &&
-std::find(ForEachMacros.begin(), ForEachMacros.end(),
-  FormatTok->Tok.getIdentifierInfo()) != ForEachMacros.end()) {
-  FormatTok->Type = TT_ForEachMacro;
+(it = std::find(Macros.begin(), Macros.end(),
+FormatTok->Tok.getIdentifierInfo())) != Macros.end()) {

Typz wrote:
> djasper wrote:
> > This does a binary search. Why aren't you implementing it with a hashtable?
> It was already done this way, so I did not change it to avoid any impact on 
> performance.
> But I can change it if you prefer.
Thanks. This is much better than it was before.

Comment at: lib/Format/FormatTokenLexer.cpp:630
   if (Style.isCpp()) {
+decltype(Macros)::iterator it;
 if (!(Tokens.size() > 0 && Tokens.back()->Tok.getIdentifierInfo() &&

Just do:

  auto it = Macros.find(FormatTok->Tok.getIdentifierInfo());
  ..

I know that this means that we might do the hash look up when we wouldn't need 
to (when we are actually in a #define), but I think clarity here is more 
important than the tiny performance benefit.

Comment at: lib/Format/FormatTokenLexer.h:25
 #include "llvm/Support/Regex.h"
+#include 

Use ".

Comment at: lib/Format/UnwrappedLineParser.cpp:1254

+  if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+nextToken();

This contains the exact same code (I think). Can you pull it out into a 
function?

https://reviews.llvm.org/D33440

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

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

2017-11-09 Thread Francois Ferrand via Phabricator via cfe-commits

Typz added a comment.

ping ?


https://reviews.llvm.org/D33589



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

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

2017-11-09 Thread Manuel Klimek via Phabricator via cfe-commits

klimek added a comment.

In https://reviews.llvm.org/D32478#806757, @Typz wrote:

> I renamed the option to `AlignAfterOperator`, is it acceptable now?
>  (I also thought of `UnindentOperator`, but I think it is better to keep the 
> notion of alignment)
>
> Note that this style is used in multiple open-source projects: Skia 
> , parts of QtCreator 
>  code base 
> (e.g. in multiple plugins: clang, cpptools, android...), the OpenEXR 
>  library...

Sorry for the long response time. I don't see this style rule expressed 
explicitly in the Skia or QtCreator style guides - is this something that just 
happens to be done sometimes in the code bases? I have problems understanding 
the rules from the discussion (as well as the code / test tbh), is there a 
really concise way to give me the core of the idea?

https://reviews.llvm.org/D32478

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

[PATCH] D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

2017-11-09 Thread Manuel Klimek via Phabricator via cfe-commits

klimek accepted this revision.
klimek added a comment.

lg




Comment at: lib/Tooling/CompilationDatabase.cpp:312
+FixedCompilationDatabase::loadFromFile(StringRef Path, std::string ) {
+  ErrorMsg.clear();
+  llvm::ErrorOr File =

sammccall wrote:
> klimek wrote:
> > I assume we can't use ErrorOr as return value?
> Again this mirrors JSONCompilationDatabase. (And also 
> CompilationDatabasePlugin, which is harder to change)
> Change this one, change both, or leave as-is?
I think we want to add a FIXME to change the interface.


https://reviews.llvm.org/D39799



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

[PATCH] D39834: [ClangDriver] -foptimization-record-file= should imply -fsave-optimization-record

2017-11-09 Thread Dmitry Venikov via Phabricator via cfe-commits

Quolyk created this revision.
Herald added a subscriber: ilya-biryukov.

This is my first attempt to contribute to llvm. I'm trying to implement this 
https://bugs.llvm.org/show_bug.cgi?id=33670. I'm struggling with writing tests 
for this patch. I will be very thankful if somebody guides me trough writing 
tests for such thing. Thanks a lot.


https://reviews.llvm.org/D39834

Files:
  lib/Driver/ToolChains/Clang.cpp


Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4376,6 +4376,11 @@
   llvm::sys::path::replace_extension(F, "opt.yaml");
   CmdArgs.push_back(Args.MakeArgString(F));
 }
+  } else {
+if (const Arg *A = 
Args.getLastArg(options::OPT_foptimization_record_file_EQ)) {
+  CmdArgs.push_back("-opt-record-file");
+  CmdArgs.push_back(A->getValue());
+}
   }
 
   bool RewriteImports = Args.hasFlag(options::OPT_frewrite_imports,


Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4376,6 +4376,11 @@
   llvm::sys::path::replace_extension(F, "opt.yaml");
   CmdArgs.push_back(Args.MakeArgString(F));
 }
+  } else {
+if (const Arg *A = Args.getLastArg(options::OPT_foptimization_record_file_EQ)) {
+  CmdArgs.push_back("-opt-record-file");
+  CmdArgs.push_back(A->getValue());
+}
   }
 
   bool RewriteImports = Args.hasFlag(options::OPT_frewrite_imports,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r317777 - [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

2017-11-09 Thread Sam McCall via cfe-commits

Author: sammccall
Date: Thu Nov  9 02:37:39 2017
New Revision: 31

URL: http://llvm.org/viewvc/llvm-project?rev=31=rev
Log:
[Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

Summary:
This is an alternative to JSONCompilationDatabase for simple projects that
don't use a build system such as CMake.
(You can also drop one in ~, to make your tools use e.g. C++11 by default)

There's no facility for varying flags per-source-file or per-machine.
Possibly this could be accommodated backwards-compatibly using cpp, but even if
not the simplicity seems worthwhile for the cases that are addressed.

Tested with clangd, works great! (requires clangd restart)

Reviewers: klimek

Subscribers: ilya-biryukov, cfe-commits

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

Added:
cfe/trunk/test/Tooling/Inputs/fixed-header.h
cfe/trunk/test/Tooling/fixed-database.cpp
Modified:
cfe/trunk/docs/JSONCompilationDatabase.rst
cfe/trunk/include/clang/Tooling/CompilationDatabase.h
cfe/trunk/lib/Tooling/CompilationDatabase.cpp

Modified: cfe/trunk/docs/JSONCompilationDatabase.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/JSONCompilationDatabase.rst?rev=31=317776=31=diff
==
--- cfe/trunk/docs/JSONCompilationDatabase.rst (original)
+++ cfe/trunk/docs/JSONCompilationDatabase.rst Thu Nov  9 02:37:39 2017
@@ -91,3 +91,9 @@ The convention is to name the file compi
 the top of the build directory. Clang tools are pointed to the top of
 the build directory to detect the file and use the compilation database
 to parse C++ code in the source tree.
+
+Alternatives
+
+For simple projects, Clang tools also recognize a compile_flags.txt file.
+This should contain one flag per line. The same flags will be used to compile
+any file.

Modified: cfe/trunk/include/clang/Tooling/CompilationDatabase.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/CompilationDatabase.h?rev=31=317776=31=diff
==
--- cfe/trunk/include/clang/Tooling/CompilationDatabase.h (original)
+++ cfe/trunk/include/clang/Tooling/CompilationDatabase.h Thu Nov  9 02:37:39 
2017
@@ -182,6 +182,11 @@ public:
   int , const char *const *Argv, std::string ,
   Twine Directory = ".");
 
+  /// Reads flags from the given file, one-per line.
+  /// Returns nullptr and sets ErrorMessage if we can't read the file.
+  static std::unique_ptr
+  loadFromFile(StringRef Path, std::string );
+
   /// \brief Constructs a compilation data base from a specified directory
   /// and command line.
   FixedCompilationDatabase(Twine Directory, ArrayRef CommandLine);

Modified: cfe/trunk/lib/Tooling/CompilationDatabase.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/CompilationDatabase.cpp?rev=31=317776=31=diff
==
--- cfe/trunk/lib/Tooling/CompilationDatabase.cpp (original)
+++ cfe/trunk/lib/Tooling/CompilationDatabase.cpp Thu Nov  9 02:37:39 2017
@@ -10,6 +10,9 @@
 //  This file contains implementations of the CompilationDatabase base class
 //  and the FixedCompilationDatabase.
 //
+//  FIXME: Various functions that take a string  should be 
upgraded
+//  to Expected.
+//
 
//===--===//
 
 #include "clang/Tooling/CompilationDatabase.h"
@@ -26,6 +29,7 @@
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Support/Host.h"
+#include "llvm/Support/LineIterator.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
@@ -302,8 +306,22 @@ FixedCompilationDatabase::loadFromComman
   std::vector StrippedArgs;
   if (!stripPositionalArgs(CommandLine, StrippedArgs, ErrorMsg))
 return nullptr;
-  return std::unique_ptr(
-  new FixedCompilationDatabase(Directory, StrippedArgs));
+  return llvm::make_unique(Directory, StrippedArgs);
+}
+
+std::unique_ptr
+FixedCompilationDatabase::loadFromFile(StringRef Path, std::string ) {
+  ErrorMsg.clear();
+  llvm::ErrorOr File =
+  llvm::MemoryBuffer::getFile(Path);
+  if (std::error_code Result = File.getError()) {
+ErrorMsg = "Error while opening fixed database: " + Result.message();
+return nullptr;
+  }
+  std::vector Args{llvm::line_iterator(**File),
+llvm::line_iterator()};
+  return llvm::make_unique(
+  llvm::sys::path::parent_path(Path), std::move(Args));
 }
 
 FixedCompilationDatabase::
@@ -334,6 +352,22 @@ FixedCompilationDatabase::getAllCompileC
   return std::vector();
 }
 
+namespace {
+
+class FixedCompilationDatabasePlugin : public CompilationDatabasePlugin {
+  std::unique_ptr
+  loadFromDirectory(StringRef Directory, std::string ) override {
+SmallString<1024>

[PATCH] D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

2017-11-09 Thread Sam McCall via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL31: [Tooling] Use FixedCompilationDatabase when 
`compile_flags.txt` is found. (authored by sammccall).

Repository:
  rL LLVM

https://reviews.llvm.org/D39799

Files:
  cfe/trunk/docs/JSONCompilationDatabase.rst
  cfe/trunk/include/clang/Tooling/CompilationDatabase.h
  cfe/trunk/lib/Tooling/CompilationDatabase.cpp
  cfe/trunk/test/Tooling/Inputs/fixed-header.h
  cfe/trunk/test/Tooling/fixed-database.cpp

Index: cfe/trunk/lib/Tooling/CompilationDatabase.cpp
===
--- cfe/trunk/lib/Tooling/CompilationDatabase.cpp
+++ cfe/trunk/lib/Tooling/CompilationDatabase.cpp
@@ -10,6 +10,9 @@
 //  This file contains implementations of the CompilationDatabase base class
 //  and the FixedCompilationDatabase.
 //
+//  FIXME: Various functions that take a string  should be upgraded
+//  to Expected.
+//
 //===--===//
 
 #include "clang/Tooling/CompilationDatabase.h"
@@ -26,6 +29,7 @@
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Support/Host.h"
+#include "llvm/Support/LineIterator.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
@@ -302,8 +306,22 @@
   std::vector StrippedArgs;
   if (!stripPositionalArgs(CommandLine, StrippedArgs, ErrorMsg))
 return nullptr;
-  return std::unique_ptr(
-  new FixedCompilationDatabase(Directory, StrippedArgs));
+  return llvm::make_unique(Directory, StrippedArgs);
+}
+
+std::unique_ptr
+FixedCompilationDatabase::loadFromFile(StringRef Path, std::string ) {
+  ErrorMsg.clear();
+  llvm::ErrorOr File =
+  llvm::MemoryBuffer::getFile(Path);
+  if (std::error_code Result = File.getError()) {
+ErrorMsg = "Error while opening fixed database: " + Result.message();
+return nullptr;
+  }
+  std::vector Args{llvm::line_iterator(**File),
+llvm::line_iterator()};
+  return llvm::make_unique(
+  llvm::sys::path::parent_path(Path), std::move(Args));
 }
 
 FixedCompilationDatabase::
@@ -334,6 +352,22 @@
   return std::vector();
 }
 
+namespace {
+
+class FixedCompilationDatabasePlugin : public CompilationDatabasePlugin {
+  std::unique_ptr
+  loadFromDirectory(StringRef Directory, std::string ) override {
+SmallString<1024> DatabasePath(Directory);
+llvm::sys::path::append(DatabasePath, "compile_flags.txt");
+return FixedCompilationDatabase::loadFromFile(DatabasePath, ErrorMessage);
+  }
+};
+
+static CompilationDatabasePluginRegistry::Add
+X("fixed-compilation-database", "Reads plain-text flags file");
+
+} // namespace
+
 namespace clang {
 namespace tooling {
 
Index: cfe/trunk/docs/JSONCompilationDatabase.rst
===
--- cfe/trunk/docs/JSONCompilationDatabase.rst
+++ cfe/trunk/docs/JSONCompilationDatabase.rst
@@ -91,3 +91,9 @@
 the top of the build directory. Clang tools are pointed to the top of
 the build directory to detect the file and use the compilation database
 to parse C++ code in the source tree.
+
+Alternatives
+
+For simple projects, Clang tools also recognize a compile_flags.txt file.
+This should contain one flag per line. The same flags will be used to compile
+any file.
Index: cfe/trunk/include/clang/Tooling/CompilationDatabase.h
===
--- cfe/trunk/include/clang/Tooling/CompilationDatabase.h
+++ cfe/trunk/include/clang/Tooling/CompilationDatabase.h
@@ -182,6 +182,11 @@
   int , const char *const *Argv, std::string ,
   Twine Directory = ".");
 
+  /// Reads flags from the given file, one-per line.
+  /// Returns nullptr and sets ErrorMessage if we can't read the file.
+  static std::unique_ptr
+  loadFromFile(StringRef Path, std::string );
+
   /// \brief Constructs a compilation data base from a specified directory
   /// and command line.
   FixedCompilationDatabase(Twine Directory, ArrayRef CommandLine);
Index: cfe/trunk/test/Tooling/fixed-database.cpp
===
--- cfe/trunk/test/Tooling/fixed-database.cpp
+++ cfe/trunk/test/Tooling/fixed-database.cpp
@@ -0,0 +1,18 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/Src
+// RUN: cp "%s" "%t/Src/test.cpp"
+// RUN: mkdir -p %t/Include
+// RUN: cp "%S/Inputs/fixed-header.h" "%t/Include/"
+// -I flag is relative to %t (where compile_flags is), not Src/.
+// RUN: echo '-IInclude/' >> %t/compile_flags.txt
+// RUN: echo "-Dklazz=class" >> %t/compile_flags.txt
+// RUN: echo '-std=c++11' >> %t/compile_flags.txt
+// RUN: clang-check "%t/Src/test.cpp" 2>&1
+// RUN: not clang-check "%s" 2>&1 | FileCheck "%s" -check-prefix=NODB
+
+// NODB: unknown type name 'klazz'
+klazz F{};
+
+// NODB: 'fixed-header.h' file not found
+#include "fixed-header.h"
+static_assert(SECRET_SYMBOL ==

[PATCH] D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

2017-11-09 Thread Sam McCall via Phabricator via cfe-commits

sammccall added a comment.

In https://reviews.llvm.org/D39799#919508, @ilya-biryukov wrote:

> In https://reviews.llvm.org/D39799#919494, @sammccall wrote:
>
> > e.g. IIUC, things like `#include "sibling.h"` won't look for the h file 
> > next to the cc file?
>
>
> Actually, this should work as quoted includes are always searched relative to 
> the current file before looking in the include paths. No matter what the WD 
> is.


Right, thanks :-)

>> Ideally yes... i'm not sure this is so easy with the registry mechanism 
>> though :-(
>>  They'd have to be present at the same level, so I'm not sure this is a huge 
>> deal.
> 
> Yeah, looks like this would require unifying `FixedCompilationDatabase` and 
> `JSONCompilationDatabase`. This doesn't seem too crazy, though. "A class that 
> handles reading compilation arguments from the filesystem." seems like a 
> valid entity to me.

It's doable, but then you're fighting the infrastructure here. The same 
ambiguity can exist between any pair of CompilationDatabasePlugins (try dumping 
a compile_commands.json in a google source tree). If we want to make this an 
error or have well-defined priority, that should be handled by 
autoDetectFromDirectory I think.

(Happy to do that if you think it's important - personally I'm not convinced it 
will be an issue in practice)


https://reviews.llvm.org/D39799



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

[PATCH] D39829: add new check for property declaration

2017-11-09 Thread Haojian Wu via Phabricator via cfe-commits

hokein added a comment.

Next time, please remember to add `cfe-commits` to the subscriber.

Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:14
+
+#include
+

Do we need this header?

Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:25
+/// we will do best effort to generate a fix, however, if the
+/// case can not be solved with a simple fix (e.g. remove prefix or change
first
+/// character), we will leave the fix to the user.

I might miss some background context.

The fix of the check seems to me that it does more things it should. It removes
all the non-alphabetical prefix characters, I'd be conservative of the fix here
(just fix the case "CamelCase", and only give a warning for other cases).

Comment at: docs/clang-tidy/checks/objc-property-declaration.rst:7
+Finds property declarations in Objective-C files that do not follow the pattern
+of property names in Google's Objective-C Style Guide. The property name should
+be in the format of Lower Camel Case.

Google style? but the link you provided is Apple.

Comment at: docs/clang-tidy/checks/objc-property-declaration.rst:27
+The corresponding style rule:
https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/CodingGuidelines/Articles/NamingIvarsAndTypes.html#//apple_ref/doc/uid/20001284-1001757
\ No newline at end of file

1 2 >

1 - 100 of 152 matches

Mail list logo