[PATCH] D42493: [clang-format] Fix ObjC message arguments formatting.

[Jacek Olesiak via Phabricator via cfe-commits]

jolesiak added inline comments.



Comment at: lib/Format/TokenAnnotator.cpp:419
   StartsObjCMethodExpr = true;
+  Left->ParameterCount = 0;
   Contexts.back().ColonIsObjCMethodExpr = true;

benhamilton wrote:
> What does this line do? Seems like it's initialized to 0 already, right?
It is indeed initialized to 0.
However, before 'Left' bracket is recognized as TT_ObjCMethodExpr it has a 
different type assigned. Hence it gets updated here:
https://github.com/llvm-mirror/clang/blob/release_60/lib/Format/TokenAnnotator.cpp#L495
This assignment is due to fact that for other languages number of parameters is 
calculated as (1 + number_of_commas).


Repository:
  rC Clang

https://reviews.llvm.org/D42493



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

[PATCH] D42395: [clang-format] Fix bug where -dump-config failed on ObjC header

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir accepted this revision.
krasimir added inline comments.
This revision is now accepted and ready to land.



Comment at: test/Format/lit.local.cfg:1
+# Suffixes supported by clang-format.
+config.suffixes = ['.c', '.cc', '.cpp', '.h', '.m', '.mm', '.java', '.js',

benhamilton wrote:
> krasimir wrote:
> > Why is this needed?
> '.h' is not in the list of file suffixes which lit is configured to look for:
> 
> https://github.com/llvm-mirror/clang/blob/master/test/lit.cfg.py#L28
> 
> So, we need to provide our own list of file suffixes here.
> 
> Other tests do the same:
> 
> https://github.com/llvm-mirror/clang/search?utf8=%E2%9C%93&q=%22config.suffixes%22&type=
> 
> so I think it's OK to do so here as well.
Thank you!


Repository:
  rC Clang

https://reviews.llvm.org/D42395



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

[PATCH] D42575: [clangd] Better handling symbols defined in macros.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clangd/index/SymbolCollector.cpp:108
 
+std::string PrintableLoc(SourceLocation Loc, SourceManager &SM) {
+  if (Loc.isInvalid())

`PrintLoc`? `PrintableLoc` sounds like a checker for whether a location is 
printable.



Comment at: clangd/index/SymbolCollector.cpp:110
+  if (Loc.isInvalid())
+return "Invalid location";
+  std::string Buffer;

This works for the use case below (i.e. to get around "getLocation().isMacroID() && llvm::StringRef(PrintableLoc(Loc, 
SM)).startswith("
+  offsetRange(llvm::StringRef Name = "") const;
+

This doesn't seem to be necessary? I think you could use the `range` method 
above and convert it to offsets in the test matcher.



Comment at: unittests/clangd/SymbolCollectorTests.cpp:216-219
+$expansion[[FF(abc)]];
+
+#define FF2() \
+  $spelling[[class Test {}]];

Consider adding another expansion and spelling in the main file (with 
`IndexMainFiles` enabled), so that we could also check file paths are handled 
correctly?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42575



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

[PATCH] D41537: Optionally add code completion results for arrow instead of dot

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan marked 4 inline comments as done.
yvvan added a comment.

@ilya-biryukov
Thanks a lot for you comments and for the provided code replacement. I'm 
checking now how it works and will continue addressing other comments.


https://reviews.llvm.org/D41537



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

[PATCH] D42638: [clangd] Add a fallback directory for collected symbols with relative paths.

[Eric Liu via Phabricator via cfe-commits]

ioeric created this revision.
ioeric added reviewers: hokein, sammccall.
Herald added subscribers: cfe-commits, jkorous-apple, ilya-biryukov, klimek.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42638

Files:
  clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolCollector.h
  unittests/clangd/SymbolCollectorTests.cpp

Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -44,6 +44,7 @@
   return arg.CompletionSnippetInsertText == S;
 }
 MATCHER_P(QName, Name, "") { return (arg.Scope + arg.Name).str() == Name; }
+MATCHER_P(Path, P, "") { return arg.CanonicalDeclaration.FilePath == P; }
 
 namespace clang {
 namespace clangd {
@@ -145,18 +146,25 @@
   runSymbolCollector(Header, Main);
   EXPECT_THAT(Symbols,
   UnorderedElementsAreArray(
-  {QName("Foo"),
-   QName("f1"),
-   QName("f2"),
-   QName("KInt"),
-   QName("kStr"),
-   QName("foo"),
-   QName("foo::bar"),
-   QName("foo::int32"),
-   QName("foo::int32_t"),
-   QName("foo::v1"),
-   QName("foo::bar::v2"),
-   QName("foo::baz")}));
+  {QName("Foo"), QName("f1"), QName("f2"), QName("KInt"),
+   QName("kStr"), QName("foo"), QName("foo::bar"),
+   QName("foo::int32"), QName("foo::int32_t"), QName("foo::v1"),
+   QName("foo::bar::v2"), QName("foo::baz")}));
+}
+
+TEST_F(SymbolCollectorTest, SymbolRelativeNoFallback) {
+  CollectorOpts.IndexMainFiles = false;
+  runSymbolCollector("class Foo {};", /*Main=*/"");
+  EXPECT_THAT(Symbols,
+  UnorderedElementsAre(AllOf(QName("Foo"), Path("symbols.h";
+}
+
+TEST_F(SymbolCollectorTest, SymbolRelativeWithFallback) {
+  CollectorOpts.IndexMainFiles = false;
+  CollectorOpts.FallbackDir = "/cwd";
+  runSymbolCollector("class Foo {};", /*Main=*/"");
+  EXPECT_THAT(Symbols, UnorderedElementsAre(
+   AllOf(QName("Foo"), Path("/cwd/symbols.h";
 }
 
 TEST_F(SymbolCollectorTest, IncludeEnums) {
Index: clangd/index/SymbolCollector.h
===
--- clangd/index/SymbolCollector.h
+++ clangd/index/SymbolCollector.h
@@ -30,6 +30,10 @@
 /// Whether to collect symbols in main files (e.g. the source file
 /// corresponding to a TU).
 bool IndexMainFiles = false;
+// When symbol paths cannot be resolved to absolute paths (e.g. files in
+// VFS that does not have absolute path), combine the fallback directory
+// with symbols' paths to get absolute paths. This must be an absolute path.
+std::string FallbackDir;
   };
 
   SymbolCollector(Options Opts);
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -14,44 +14,51 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Index/IndexSymbol.h"
 #include "clang/Index/USRGeneration.h"
+#include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 
 namespace clang {
 namespace clangd {
 
 namespace {
 // Make the Path absolute using the current working directory of the given
-// SourceManager if the Path is not an absolute path.
+// SourceManager if the Path is not an absolute path. If failed, this combine
+// relative paths with \p FallbackDir to get an absolute path.
 //
 // The Path can be a path relative to the build directory, or retrieved from
 // the SourceManager.
-std::string makeAbsolutePath(const SourceManager &SM, StringRef Path) {
+std::string makeAbsolutePath(const SourceManager &SM, StringRef Path,
+ StringRef FallbackDir) {
   llvm::SmallString<128> AbsolutePath(Path);
   if (std::error_code EC =
   SM.getFileManager().getVirtualFileSystem()->makeAbsolute(
   AbsolutePath))
 llvm::errs() << "Warning: could not make absolute file: '" << EC.message()
  << '\n';
-  // Handle the symbolic link path case where the current working directory
-  // (getCurrentWorkingDirectory) is a symlink./ We always want to the real
-  // file path (instead of the symlink path) for the  C++ symbols.
-  //
-  // Consider the following example:
-  //
-  //   src dir: /project/src/foo.h
-  //   current working directory (symlink): /tmp/build -> /project/src/
-  //
-  // The file path of Symbol is "/project/src/foo.h" instead of
-  // "/tmp/build/foo.h"
-  const DirectoryEntry *Dir = SM.getFileManager().getDirectory(
-  llvm::sys::path::parent_path(AbsolutePath.str()));
-  if (Dir) {
-StringRef DirName = SM.getFil

[PATCH] D42174: [clangd] Refactored threading in ClangdServer

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/ClangdServer.h:177
+/// preambles and ASTs) for opened files.
+class Scheduler {
+public:

ilya-biryukov wrote:
> ilya-biryukov wrote:
> > sammccall wrote:
> > > sammccall wrote:
> > > > This class has important responsibilities beyond threading itself, 
> > > > which "Scheduler" suggests.
> > > > 
> > > > I can't think of a perfectly coherent name, options that seem 
> > > > reasonable:
> > > >  - TUManager - pretty bland/vague, but gets what this class is mostly 
> > > > about
> > > >  - Workshop - kind of a silly metaphor, but unlikely to be confused 
> > > > with something else
> > > >  - Clearinghouse - another silly metaphor, maybe more accurate but more 
> > > > obscure
> > > Worth saying something abouth the threading properties here:
> > > 
> > > - Scheduler is not threadsafe, only the main thread should be providing 
> > > updates and scheduling tasks.
> > >  - callbacks are run on a large threadpool, and it's appropriate to do 
> > > slow, blocking work in them
> > Added comments.
> It'd be nice to have some mention of the fact that the class handles 
> threading responsibilities. None of the options seem to capture this.
> I don't have good suggestions either, though.
> 
Rename of the Scheduler seems to be the only thing blocking this patch from 
landing.
I'm happy to go with either of the suggested alternatives or leave as is, I 
couldn't come up with anything better.

@sammccall, what option would you prefer?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42174



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

[PATCH] D41720: [clang-tidy] Add a -show-color flag.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/tool/ClangTidyMain.cpp:150-152
+Show color diagnostics. If not specified,
+defaults to on when a color-capable terminal
+is detected.)"),

itessier wrote:
> itessier wrote:
> > aaron.ballman wrote:
> > > I think this raw string can be reflowed a bit?
> > > 
> > > Instead of "defaults to on", how about "Defaults to true when a 
> > > color-capable terminal is detected."?
> > I copied that part from the -fcolor-diagnostics flag to be consistent. I 
> > don't changing "on" to "true" if you prefer that instead.
> That should have read "I don't mind changing..."
I'd say let's go ahead and change it, and once this patch lands, change the 
help text for `-fcolor-diagnostics` in a follow-up.


https://reviews.llvm.org/D41720



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

[PATCH] D42419: [clangd] Use new URI with scheme support in place of the existing LSP URI

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 131775.
ioeric added a comment.

- Merge remote-tracking branch 'origin/master' into uri


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42419

Files:
  clangd/ClangdLSPServer.cpp
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/URI.cpp
  clangd/URI.h
  clangd/XRefs.cpp
  unittests/clangd/URITests.cpp
  unittests/clangd/XRefsTests.cpp

Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -9,6 +9,7 @@
 #include "Annotations.h"
 #include "ClangdUnit.h"
 #include "Matchers.h"
+#include "TestFS.h"
 #include "XRefs.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/PCHContainerOperations.h"
@@ -38,7 +39,9 @@
 
 // FIXME: this is duplicated with FileIndexTests. Share it.
 ParsedAST build(StringRef Code) {
-  auto CI = createInvocationFromCommandLine({"clang", "-xc++", "Foo.cpp"});
+  auto TestFile = getVirtualTestFilePath("Foo.cpp");
+  auto CI =
+  createInvocationFromCommandLine({"clang", "-xc++", TestFile.c_str()});
   auto Buf = MemoryBuffer::getMemBuffer(Code);
   auto AST = ParsedAST::Build(
   Context::empty(), std::move(CI), nullptr, std::move(Buf),
Index: unittests/clangd/URITests.cpp
===
--- unittests/clangd/URITests.cpp
+++ unittests/clangd/URITests.cpp
@@ -40,13 +40,12 @@
 .str();
   }
 
-  llvm::Expected
+  llvm::Expected
   uriFromAbsolutePath(llvm::StringRef AbsolutePath) const override {
 auto Pos = AbsolutePath.find(TestRoot);
 assert(Pos != llvm::StringRef::npos);
-return FileURI::create(
-Scheme, /*Authority=*/"",
-AbsolutePath.substr(Pos + llvm::StringRef(TestRoot).size()));
+return URI(Scheme, /*Authority=*/"",
+   AbsolutePath.substr(Pos + llvm::StringRef(TestRoot).size()));
   }
 };
 
@@ -57,30 +56,22 @@
 
 std::string createOrDie(llvm::StringRef AbsolutePath,
 llvm::StringRef Scheme = "file") {
-  auto Uri = FileURI::create(AbsolutePath, Scheme);
+  auto Uri = URI::create(AbsolutePath, Scheme);
   if (!Uri)
 llvm_unreachable(llvm::toString(Uri.takeError()).c_str());
   return Uri->toString();
 }
 
-std::string createOrDie(llvm::StringRef Scheme, llvm::StringRef Authority,
-llvm::StringRef Body) {
-  auto Uri = FileURI::create(Scheme, Authority, Body);
-  if (!Uri)
-llvm_unreachable(llvm::toString(Uri.takeError()).c_str());
-  return Uri->toString();
-}
-
-FileURI parseOrDie(llvm::StringRef Uri) {
-  auto U = FileURI::parse(Uri);
+URI parseOrDie(llvm::StringRef Uri) {
+  auto U = URI::parse(Uri);
   if (!U)
 llvm_unreachable(llvm::toString(U.takeError()).c_str());
   return *U;
 }
 
 TEST(PercentEncodingTest, Encode) {
-  EXPECT_EQ(createOrDie("x", /*Authority=*/"", "a/b/c"), "x:a/b/c");
-  EXPECT_EQ(createOrDie("x", /*Authority=*/"", "a!b;c~"), "x:a%21b%3bc~");
+  EXPECT_EQ(URI("x", /*Authority=*/"", "a/b/c").toString(), "x:a/b/c");
+  EXPECT_EQ(URI("x", /*Authority=*/"", "a!b;c~").toString(), "x:a%21b%3bc~");
 }
 
 TEST(PercentEncodingTest, Decode) {
@@ -93,8 +84,8 @@
   EXPECT_EQ(parseOrDie("x:a%21b%3ac~").body(), "a!b:c~");
 }
 
-std::string resolveOrDie(const FileURI &U, llvm::StringRef HintPath = "") {
-  auto Path = FileURI::resolve(U, HintPath);
+std::string resolveOrDie(const URI &U, llvm::StringRef HintPath = "") {
+  auto Path = URI::resolve(U, HintPath);
   if (!Path)
 llvm_unreachable(llvm::toString(Path.takeError()).c_str());
   return *Path;
@@ -110,25 +101,16 @@
 }
 
 TEST(URITest, FailedCreate) {
-  auto Fail = [](llvm::Expected U) {
+  auto Fail = [](llvm::Expected U) {
 if (!U) {
   llvm::consumeError(U.takeError());
   return true;
 }
 return false;
   };
-  // Create from scheme+authority+body:
-  //
-  // Scheme must be provided.
-  EXPECT_TRUE(Fail(FileURI::create("", "auth", "/a")));
-  // Body must start with '/' if authority is present.
-  EXPECT_TRUE(Fail(FileURI::create("scheme", "auth", "x/y/z")));
-
-  // Create from scheme registry:
-  //
-  EXPECT_TRUE(Fail(FileURI::create("/x/y/z", "no")));
+  EXPECT_TRUE(Fail(URI::create("/x/y/z", "no")));
   // Path has to be absolute.
-  EXPECT_TRUE(Fail(FileURI::create("x/y/z")));
+  EXPECT_TRUE(Fail(URI::create("x/y/z", "file")));
 }
 
 TEST(URITest, Parse) {
@@ -163,7 +145,7 @@
 
 TEST(URITest, ParseFailed) {
   auto FailedParse = [](llvm::StringRef U) {
-auto URI = FileURI::parse(U);
+auto URI = URI::parse(U);
 if (!URI) {
   llvm::consumeError(URI.takeError());
   return true;
@@ -194,14 +176,14 @@
 
 TEST(URITest, Platform) {
   auto Path = getVirtualTestFilePath("x");
-  auto U = FileURI::create(Path, "file");
+  auto U = URI::create(Path, "file");
   EXPECT_TRUE(static_cast(U));
   EXPECT_THAT(resolveOrDie(*U), Path.str());
 }
 
 TEST(URITest, ResolveFailed) {
   auto FailedResolve

[PATCH] D42623: [clang-tidy] Add a Lexer util to get the source text of a statement

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/utils/LexerUtils.cpp:39
+Optional getStmtText(const Stmt* Statement, const SourceManager& 
SM) {
+  if (!Statement) {
+return llvm::NoneType();

You should elide the curly braces here.



Comment at: clang-tidy/utils/LexerUtils.cpp:44
+  bool Error = false;
+  auto Ret = Lexer::getSourceText(
+CharSourceRange::getTokenRange(Statement->getSourceRange()),

Please don't use `auto` here, as the type is not spelled out in the 
initialization.



Comment at: clang-tidy/utils/LexerUtils.h:26
+/// Get source code text for statement.
+Optional getStmtText(const Stmt* Statement, const SourceManager& 
SM);
+

Formatting (elsewhere as well).

You should run the patch under clang-format to handle this sort of thing.



Comment at: unittests/clang-tidy/LexerUtilsTest.cpp:31
+TEST_F(GetStmtTextTest, SimpleStatement) {
+  auto FooIdent = &Unit->getASTContext().Idents.getOwn("foo");
+  auto FooDeclName = 
Unit->getASTContext().DeclarationNames.getIdentifier(FooIdent);

Don't use `auto` unless the type is spelled out in the init, such as with 
`dyn_cast`



Comment at: unittests/clang-tidy/LexerUtilsTest.cpp:34-35
+  auto FooLookup = 
Unit->getASTContext().getTranslationUnitDecl()->lookup(FooDeclName);
+  auto FooFunction = dyn_cast_or_null(FooLookup.front());
+  EXPECT_TRUE(FooFunction);
+

Can `FooLookup.front()` actually be null? I think this should use `cast<>` and 
assert if given null or something other than a `FunctionDecl`.



Comment at: unittests/clang-tidy/LexerUtilsTest.cpp:37-38
+
+  auto FooBody = dyn_cast_or_null(FooFunction->getBody());
+  EXPECT_TRUE(FooBody);
+  EXPECT_TRUE(FooBody->body_begin());

I think you can use `cast<>` here instead of `dyn_cast_or_null<>`, and it will 
assert for you if the body isn't valid.



Comment at: unittests/clang-tidy/LexerUtilsTest.cpp:43
+
+
+  auto Res = utils::lexer::getStmtText(DeclStmt, SM);

Spurious newline.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42623



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

[PATCH] D42577: [Lexer] Support adding working directory to relative search dir for #include shortening in HeaderSearch.

[Benjamin Kramer via Phabricator via cfe-commits]

bkramer accepted this revision.
bkramer added inline comments.
This revision is now accepted and ready to land.



Comment at: include/clang/Lex/HeaderSearch.h:708
+  ///
+  /// \param WorkingDir If non-empty, this will be prepend to search directory
+  /// paths that are relative.

prepended



Comment at: lib/Lex/HeaderSearch.cpp:1584
+  // the most appropriate one for this analysis (and that it's spelled the
+  // same
   // way as the corresponding header search path).

Reflow comment.


Repository:
  rC Clang

https://reviews.llvm.org/D42577



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

[PATCH] D42174: [clangd] Refactored threading in ClangdServer

[Sam McCall via Phabricator via cfe-commits]

sammccall added a subscriber: hokein.
sammccall added inline comments.



Comment at: clangd/ClangdServer.h:177
+/// preambles and ASTs) for opened files.
+class Scheduler {
+public:

ilya-biryukov wrote:
> ilya-biryukov wrote:
> > ilya-biryukov wrote:
> > > sammccall wrote:
> > > > sammccall wrote:
> > > > > This class has important responsibilities beyond threading itself, 
> > > > > which "Scheduler" suggests.
> > > > > 
> > > > > I can't think of a perfectly coherent name, options that seem 
> > > > > reasonable:
> > > > >  - TUManager - pretty bland/vague, but gets what this class is mostly 
> > > > > about
> > > > >  - Workshop - kind of a silly metaphor, but unlikely to be confused 
> > > > > with something else
> > > > >  - Clearinghouse - another silly metaphor, maybe more accurate but 
> > > > > more obscure
> > > > Worth saying something abouth the threading properties here:
> > > > 
> > > > - Scheduler is not threadsafe, only the main thread should be providing 
> > > > updates and scheduling tasks.
> > > >  - callbacks are run on a large threadpool, and it's appropriate to do 
> > > > slow, blocking work in them
> > > Added comments.
> > It'd be nice to have some mention of the fact that the class handles 
> > threading responsibilities. None of the options seem to capture this.
> > I don't have good suggestions either, though.
> > 
> Rename of the Scheduler seems to be the only thing blocking this patch from 
> landing.
> I'm happy to go with either of the suggested alternatives or leave as is, I 
> couldn't come up with anything better.
> 
> @sammccall, what option would you prefer?
For a descriptive name I think we need something TU-related in there. If we 
want it to cover threads too, I can't think of anything better than 
`TUScheduler`.

I'm also happy with `Workshop` or similar, which prods you to work out what the 
class actually does. Maybe other people will find this confusing. Will solicit 
opinions.

...

Chatted with @ioeric and @hokein - Workshop is too silly. Suggestions were 
along the lines of what we've been discussing: TUTaskManager or similar.
So I'd probably go with TUScheduler, it's not horrendously long and it's close 
enough to Scheduler that those of us who spent last week discussing it won't 
get confused :-)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42174



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

r323647 - [Lexer] Support adding working directory to relative search dir for #include shortening in HeaderSearch.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Mon Jan 29 05:21:23 2018
New Revision: 323647

URL: http://llvm.org/viewvc/llvm-project?rev=323647&view=rev
Log:
[Lexer] Support adding working directory to relative search dir for #include 
shortening in HeaderSearch.

Reviewers: bkramer

Subscribers: mgorny, hintonda, cfe-commits

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

Added:
cfe/trunk/unittests/Lex/HeaderSearchTest.cpp
Modified:
cfe/trunk/include/clang/Lex/HeaderSearch.h
cfe/trunk/lib/Lex/HeaderSearch.cpp
cfe/trunk/unittests/Lex/CMakeLists.txt

Modified: cfe/trunk/include/clang/Lex/HeaderSearch.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/HeaderSearch.h?rev=323647&r1=323646&r2=323647&view=diff
==
--- cfe/trunk/include/clang/Lex/HeaderSearch.h (original)
+++ cfe/trunk/include/clang/Lex/HeaderSearch.h Mon Jan 29 05:21:23 2018
@@ -693,7 +693,7 @@ public:
 
   /// \brief Retrieve a uniqued framework name.
   StringRef getUniqueFrameworkName(StringRef Framework);
-  
+
   /// \brief Suggest a path by which the specified file could be found, for
   /// use in diagnostics to suggest a #include.
   ///
@@ -702,6 +702,15 @@ public:
   std::string suggestPathToFileForDiagnostics(const FileEntry *File,
   bool *IsSystem = nullptr);
 
+  /// \brief Suggest a path by which the specified file could be found, for
+  /// use in diagnostics to suggest a #include.
+  ///
+  /// \param WorkingDir If non-empty, this will be prepended to search 
directory
+  /// paths that are relative.
+  std::string suggestPathToFileForDiagnostics(llvm::StringRef File,
+  llvm::StringRef WorkingDir,
+  bool *IsSystem = nullptr);
+
   void PrintStats();
   
   size_t getTotalMemory() const;

Modified: cfe/trunk/lib/Lex/HeaderSearch.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/HeaderSearch.cpp?rev=323647&r1=323646&r2=323647&view=diff
==
--- cfe/trunk/lib/Lex/HeaderSearch.cpp (original)
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp Mon Jan 29 05:21:23 2018
@@ -1580,9 +1580,15 @@ void HeaderSearch::loadSubdirectoryModul
 std::string HeaderSearch::suggestPathToFileForDiagnostics(const FileEntry 
*File,
   bool *IsSystem) {
   // FIXME: We assume that the path name currently cached in the FileEntry is
-  // the most appropriate one for this analysis (and that it's spelled the same
-  // way as the corresponding header search path).
-  StringRef Name = File->getName();
+  // the most appropriate one for this analysis (and that it's spelled the
+  // same way as the corresponding header search path).
+  return suggestPathToFileForDiagnostics(File->getName(), /*BuildDir=*/"",
+ IsSystem);
+}
+
+std::string HeaderSearch::suggestPathToFileForDiagnostics(
+llvm::StringRef File, llvm::StringRef WorkingDir, bool *IsSystem) {
+  using namespace llvm::sys;
 
   unsigned BestPrefixLength = 0;
   unsigned BestSearchDir;
@@ -1593,12 +1599,17 @@ std::string HeaderSearch::suggestPathToF
   continue;
 
 StringRef Dir = SearchDirs[I].getDir()->getName();
-for (auto NI = llvm::sys::path::begin(Name),
-  NE = llvm::sys::path::end(Name),
-  DI = llvm::sys::path::begin(Dir),
-  DE = llvm::sys::path::end(Dir);
+llvm::SmallString<32> DirPath(Dir.begin(), Dir.end());
+if (!WorkingDir.empty() && !path::is_absolute(Dir)) {
+  auto err = fs::make_absolute(WorkingDir, DirPath);
+  if (!err)
+path::remove_dots(DirPath, /*remove_dot_dot=*/true);
+  Dir = DirPath;
+}
+for (auto NI = path::begin(File), NE = path::end(File),
+  DI = path::begin(Dir), DE = path::end(Dir);
  /*termination condition in loop*/; ++NI, ++DI) {
-  // '.' components in Name are ignored.
+  // '.' components in File are ignored.
   while (NI != NE && *NI == ".")
 ++NI;
   if (NI == NE)
@@ -1608,9 +1619,9 @@ std::string HeaderSearch::suggestPathToF
   while (DI != DE && *DI == ".")
 ++DI;
   if (DI == DE) {
-// Dir is a prefix of Name, up to '.' components and choice of path
+// Dir is a prefix of File, up to '.' components and choice of path
 // separators.
-unsigned PrefixLength = NI - llvm::sys::path::begin(Name);
+unsigned PrefixLength = NI - path::begin(File);
 if (PrefixLength > BestPrefixLength) {
   BestPrefixLength = PrefixLength;
   BestSearchDir = I;
@@ -1625,5 +1636,5 @@ std::string HeaderSearch::suggestPathToF
 
   if (IsSystem)
 *IsSystem = BestPrefixLength ? BestSearchDir >= SystemDirIdx : false;
-  return Name.drop_front(BestPrefixLength);
+  return File.drop_fr

[PATCH] D42639: [clang-move] Clever on handling header file which includes itself.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: ioeric.
Herald added a subscriber: klimek.

Previously, we assume only old.cc includes "old.h", which would
introduce incorrect fixes for the cases where old.h also includes `#include 
"old.h"`

Although it should not be occurred in real projects, clang-move should handle 
this.

Old.h:

  class Foo {};

after moving to a new old.h:

  class Foo {};


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42639

Files:
  clang-move/ClangMove.cpp
  clang-move/ClangMove.h
  unittests/clang-move/ClangMoveTests.cpp

Index: unittests/clang-move/ClangMoveTests.cpp
===
--- unittests/clang-move/ClangMoveTests.cpp
+++ unittests/clang-move/ClangMoveTests.cpp
@@ -293,6 +293,32 @@
   EXPECT_EQ(0u, Results.size());
 }
 
+TEST(ClangMove, HeaderIncludeSelf) {
+  move::MoveDefinitionSpec Spec;
+  Spec.Names = {std::string("Foo")};
+  Spec.OldHeader = "foo.h";
+  Spec.OldCC = "foo.cc";
+  Spec.NewHeader = "new_foo.h";
+  Spec.NewCC = "new_foo.cc";
+
+  const char TestHeader[] = "#ifndef FOO_H\n"
+"#define FOO_H\n"
+"#include \"foo.h\"\n"
+"class Foo {};\n"
+"#endif\n";
+  const char TestCode[] = "#include \"foo.h\"";
+  const char ExpectedNewHeader[] = "#ifndef FOO_H\n"
+   "#define FOO_H\n"
+   "#include \"new_foo.h\"\n"
+   "class Foo {};\n"
+   "#endif\n";
+  const char ExpectedNewCC[] = "#include \"new_foo.h\"";
+  auto Results = runClangMoveOnCode(Spec, TestHeader, TestCode);
+  EXPECT_EQ("", Results[Spec.OldHeader]);
+  EXPECT_EQ(ExpectedNewHeader, Results[Spec.NewHeader]);
+  EXPECT_EQ(ExpectedNewCC, Results[Spec.NewCC]);
+}
+
 TEST(ClangMove, MoveAll) {
   std::vector TestHeaders = {
 "class A {\npublic:\n  int f();\n};",
Index: clang-move/ClangMove.h
===
--- clang-move/ClangMove.h
+++ clang-move/ClangMove.h
@@ -176,7 +176,11 @@
   /// The source range for the written file name in #include (i.e. "old.h" for
   /// #include "old.h") in old.cc,  including the enclosing quotes or angle
   /// brackets.
-  clang::CharSourceRange OldHeaderIncludeRange;
+  clang::CharSourceRange OldHeaderIncludeRangeInCC;
+  /// The source range for the written file name in #include (i.e. "old.h" for
+  /// #include "old.h") in old.h,  including the enclosing quotes or angle
+  /// brackets.
+  clang::CharSourceRange OldHeaderIncludeRangeInHeader;
   /// Mapping from FilePath to FileID, which can be used in post processes like
   /// cleanup around replacements.
   llvm::StringMap FilePathToFileID;
Index: clang-move/ClangMove.cpp
===
--- clang-move/ClangMove.cpp
+++ clang-move/ClangMove.cpp
@@ -694,22 +694,28 @@
 const SourceManager &SM) {
   SmallVector HeaderWithSearchPath;
   llvm::sys::path::append(HeaderWithSearchPath, SearchPath, IncludeHeader);
-  std::string AbsoluteOldHeader = makeAbsolutePath(Context->Spec.OldHeader);
-  if (AbsoluteOldHeader ==
+  std::string AbsoluteIncludeHeader =
   MakeAbsolutePath(SM, llvm::StringRef(HeaderWithSearchPath.data(),
-   HeaderWithSearchPath.size( {
-OldHeaderIncludeRange = IncludeFilenameRange;
-return;
-  }
-
+   HeaderWithSearchPath.size()));
   std::string IncludeLine =
   IsAngled ? ("#include <" + IncludeHeader + ">\n").str()
: ("#include \"" + IncludeHeader + "\"\n").str();
 
+  std::string AbsoluteOldHeader = makeAbsolutePath(Context->Spec.OldHeader);
   std::string AbsoluteCurrentFile = MakeAbsolutePath(SM, FileName);
   if (AbsoluteOldHeader == AbsoluteCurrentFile) {
+// Find old.h includes "old.h".
+if (AbsoluteOldHeader == AbsoluteOldHeader) {
+  OldHeaderIncludeRangeInHeader = IncludeFilenameRange;
+  return;
+}
 HeaderIncludes.push_back(IncludeLine);
   } else if (makeAbsolutePath(Context->Spec.OldCC) == AbsoluteCurrentFile) {
+// Find old.cc includes "old.h".
+if (AbsoluteOldHeader == AbsoluteIncludeHeader) {
+  OldHeaderIncludeRangeInCC = IncludeFilenameRange;
+  return;
+}
 CCIncludes.push_back(IncludeLine);
   }
 }
@@ -857,12 +863,17 @@
   if (!NewFile.empty()) {
 auto AllCode = clang::tooling::Replacements(
 clang::tooling::Replacement(NewFile, 0, 0, Code));
-// If we are moving from old.cc, an extra step is required: excluding
-// the #include of "old.h", instead, we replace it with #include of "new.h".
-if (Context->Spec.NewCC == NewFile && OldHeaderIncludeRange.isValid()) {
-  AllCode = AllCode.merge(
-  clang::tooling::Replacements(clang::tooling::Repla

[PATCH] D42577: [Lexer] Support adding working directory to relative search dir for #include shortening in HeaderSearch.

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL323647: [Lexer] Support adding working directory to relative 
search dir for #include… (authored by ioeric, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D42577

Files:
  cfe/trunk/include/clang/Lex/HeaderSearch.h
  cfe/trunk/lib/Lex/HeaderSearch.cpp
  cfe/trunk/unittests/Lex/CMakeLists.txt
  cfe/trunk/unittests/Lex/HeaderSearchTest.cpp

Index: cfe/trunk/lib/Lex/HeaderSearch.cpp
===
--- cfe/trunk/lib/Lex/HeaderSearch.cpp
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp
@@ -1580,9 +1580,15 @@
 std::string HeaderSearch::suggestPathToFileForDiagnostics(const FileEntry *File,
   bool *IsSystem) {
   // FIXME: We assume that the path name currently cached in the FileEntry is
-  // the most appropriate one for this analysis (and that it's spelled the same
-  // way as the corresponding header search path).
-  StringRef Name = File->getName();
+  // the most appropriate one for this analysis (and that it's spelled the
+  // same way as the corresponding header search path).
+  return suggestPathToFileForDiagnostics(File->getName(), /*BuildDir=*/"",
+ IsSystem);
+}
+
+std::string HeaderSearch::suggestPathToFileForDiagnostics(
+llvm::StringRef File, llvm::StringRef WorkingDir, bool *IsSystem) {
+  using namespace llvm::sys;
 
   unsigned BestPrefixLength = 0;
   unsigned BestSearchDir;
@@ -1593,12 +1599,17 @@
   continue;
 
 StringRef Dir = SearchDirs[I].getDir()->getName();
-for (auto NI = llvm::sys::path::begin(Name),
-  NE = llvm::sys::path::end(Name),
-  DI = llvm::sys::path::begin(Dir),
-  DE = llvm::sys::path::end(Dir);
+llvm::SmallString<32> DirPath(Dir.begin(), Dir.end());
+if (!WorkingDir.empty() && !path::is_absolute(Dir)) {
+  auto err = fs::make_absolute(WorkingDir, DirPath);
+  if (!err)
+path::remove_dots(DirPath, /*remove_dot_dot=*/true);
+  Dir = DirPath;
+}
+for (auto NI = path::begin(File), NE = path::end(File),
+  DI = path::begin(Dir), DE = path::end(Dir);
  /*termination condition in loop*/; ++NI, ++DI) {
-  // '.' components in Name are ignored.
+  // '.' components in File are ignored.
   while (NI != NE && *NI == ".")
 ++NI;
   if (NI == NE)
@@ -1608,9 +1619,9 @@
   while (DI != DE && *DI == ".")
 ++DI;
   if (DI == DE) {
-// Dir is a prefix of Name, up to '.' components and choice of path
+// Dir is a prefix of File, up to '.' components and choice of path
 // separators.
-unsigned PrefixLength = NI - llvm::sys::path::begin(Name);
+unsigned PrefixLength = NI - path::begin(File);
 if (PrefixLength > BestPrefixLength) {
   BestPrefixLength = PrefixLength;
   BestSearchDir = I;
@@ -1625,5 +1636,5 @@
 
   if (IsSystem)
 *IsSystem = BestPrefixLength ? BestSearchDir >= SystemDirIdx : false;
-  return Name.drop_front(BestPrefixLength);
+  return File.drop_front(BestPrefixLength);
 }
Index: cfe/trunk/unittests/Lex/CMakeLists.txt
===
--- cfe/trunk/unittests/Lex/CMakeLists.txt
+++ cfe/trunk/unittests/Lex/CMakeLists.txt
@@ -4,6 +4,7 @@
 
 add_clang_unittest(LexTests
   HeaderMapTest.cpp
+  HeaderSearchTest.cpp
   LexerTest.cpp
   PPCallbacksTest.cpp
   PPConditionalDirectiveRecordTest.cpp
Index: cfe/trunk/unittests/Lex/HeaderSearchTest.cpp
===
--- cfe/trunk/unittests/Lex/HeaderSearchTest.cpp
+++ cfe/trunk/unittests/Lex/HeaderSearchTest.cpp
@@ -0,0 +1,96 @@
+//===- unittests/Lex/HeaderSearchTest.cpp -- HeaderSearch tests ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/Lex/HeaderSearch.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/MemoryBufferCache.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TargetInfo.h"
+#include "clang/Basic/TargetOptions.h"
+#include "clang/Lex/HeaderSearch.h"
+#include "clang/Lex/HeaderSearchOptions.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace {
+
+// The test fixture.
+class HeaderSearchTest : public ::testing::Test {
+protected:
+  HeaderSearchTest()
+  : VFS(new vfs::InMemoryFileSystem), FileMgr(FileMgrOpts, VFS),
+DiagID(new DiagnosticIDs()),
+Diags(DiagID, new DiagnosticOptions, new IgnoringDiagConsum

[PATCH] D42577: [Lexer] Support adding working directory to relative search dir for #include shortening in HeaderSearch.

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC323647: [Lexer] Support adding working directory to relative 
search dir for #include… (authored by ioeric, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D42577?vs=131777&id=131781#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D42577

Files:
  include/clang/Lex/HeaderSearch.h
  lib/Lex/HeaderSearch.cpp
  unittests/Lex/CMakeLists.txt
  unittests/Lex/HeaderSearchTest.cpp

Index: include/clang/Lex/HeaderSearch.h
===
--- include/clang/Lex/HeaderSearch.h
+++ include/clang/Lex/HeaderSearch.h
@@ -693,15 +693,24 @@
 
   /// \brief Retrieve a uniqued framework name.
   StringRef getUniqueFrameworkName(StringRef Framework);
-  
+
   /// \brief Suggest a path by which the specified file could be found, for
   /// use in diagnostics to suggest a #include.
   ///
   /// \param IsSystem If non-null, filled in to indicate whether the suggested
   ///path is relative to a system header directory.
   std::string suggestPathToFileForDiagnostics(const FileEntry *File,
   bool *IsSystem = nullptr);
 
+  /// \brief Suggest a path by which the specified file could be found, for
+  /// use in diagnostics to suggest a #include.
+  ///
+  /// \param WorkingDir If non-empty, this will be prepended to search directory
+  /// paths that are relative.
+  std::string suggestPathToFileForDiagnostics(llvm::StringRef File,
+  llvm::StringRef WorkingDir,
+  bool *IsSystem = nullptr);
+
   void PrintStats();
   
   size_t getTotalMemory() const;
Index: lib/Lex/HeaderSearch.cpp
===
--- lib/Lex/HeaderSearch.cpp
+++ lib/Lex/HeaderSearch.cpp
@@ -1580,9 +1580,15 @@
 std::string HeaderSearch::suggestPathToFileForDiagnostics(const FileEntry *File,
   bool *IsSystem) {
   // FIXME: We assume that the path name currently cached in the FileEntry is
-  // the most appropriate one for this analysis (and that it's spelled the same
-  // way as the corresponding header search path).
-  StringRef Name = File->getName();
+  // the most appropriate one for this analysis (and that it's spelled the
+  // same way as the corresponding header search path).
+  return suggestPathToFileForDiagnostics(File->getName(), /*BuildDir=*/"",
+ IsSystem);
+}
+
+std::string HeaderSearch::suggestPathToFileForDiagnostics(
+llvm::StringRef File, llvm::StringRef WorkingDir, bool *IsSystem) {
+  using namespace llvm::sys;
 
   unsigned BestPrefixLength = 0;
   unsigned BestSearchDir;
@@ -1593,12 +1599,17 @@
   continue;
 
 StringRef Dir = SearchDirs[I].getDir()->getName();
-for (auto NI = llvm::sys::path::begin(Name),
-  NE = llvm::sys::path::end(Name),
-  DI = llvm::sys::path::begin(Dir),
-  DE = llvm::sys::path::end(Dir);
+llvm::SmallString<32> DirPath(Dir.begin(), Dir.end());
+if (!WorkingDir.empty() && !path::is_absolute(Dir)) {
+  auto err = fs::make_absolute(WorkingDir, DirPath);
+  if (!err)
+path::remove_dots(DirPath, /*remove_dot_dot=*/true);
+  Dir = DirPath;
+}
+for (auto NI = path::begin(File), NE = path::end(File),
+  DI = path::begin(Dir), DE = path::end(Dir);
  /*termination condition in loop*/; ++NI, ++DI) {
-  // '.' components in Name are ignored.
+  // '.' components in File are ignored.
   while (NI != NE && *NI == ".")
 ++NI;
   if (NI == NE)
@@ -1608,9 +1619,9 @@
   while (DI != DE && *DI == ".")
 ++DI;
   if (DI == DE) {
-// Dir is a prefix of Name, up to '.' components and choice of path
+// Dir is a prefix of File, up to '.' components and choice of path
 // separators.
-unsigned PrefixLength = NI - llvm::sys::path::begin(Name);
+unsigned PrefixLength = NI - path::begin(File);
 if (PrefixLength > BestPrefixLength) {
   BestPrefixLength = PrefixLength;
   BestSearchDir = I;
@@ -1625,5 +1636,5 @@
 
   if (IsSystem)
 *IsSystem = BestPrefixLength ? BestSearchDir >= SystemDirIdx : false;
-  return Name.drop_front(BestPrefixLength);
+  return File.drop_front(BestPrefixLength);
 }
Index: unittests/Lex/HeaderSearchTest.cpp
===
--- unittests/Lex/HeaderSearchTest.cpp
+++ unittests/Lex/HeaderSearchTest.cpp
@@ -0,0 +1,96 @@
+//===- unittests/Lex/HeaderSearchTest.cpp -- HeaderSearch tests ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--

[PATCH] D42577: [Lexer] Support adding working directory to relative search dir for #include shortening in HeaderSearch.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 131777.
ioeric marked 2 inline comments as done.
ioeric added a comment.

- Addressed review comments.


Repository:
  rC Clang

https://reviews.llvm.org/D42577

Files:
  include/clang/Lex/HeaderSearch.h
  lib/Lex/HeaderSearch.cpp
  unittests/Lex/CMakeLists.txt
  unittests/Lex/HeaderSearchTest.cpp

Index: unittests/Lex/HeaderSearchTest.cpp
===
--- /dev/null
+++ unittests/Lex/HeaderSearchTest.cpp
@@ -0,0 +1,96 @@
+//===- unittests/Lex/HeaderSearchTest.cpp -- HeaderSearch tests ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/Lex/HeaderSearch.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/MemoryBufferCache.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TargetInfo.h"
+#include "clang/Basic/TargetOptions.h"
+#include "clang/Lex/HeaderSearch.h"
+#include "clang/Lex/HeaderSearchOptions.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace {
+
+// The test fixture.
+class HeaderSearchTest : public ::testing::Test {
+protected:
+  HeaderSearchTest()
+  : VFS(new vfs::InMemoryFileSystem), FileMgr(FileMgrOpts, VFS),
+DiagID(new DiagnosticIDs()),
+Diags(DiagID, new DiagnosticOptions, new IgnoringDiagConsumer()),
+SourceMgr(Diags, FileMgr), TargetOpts(new TargetOptions),
+Search(std::make_shared(), SourceMgr, Diags,
+   LangOpts, Target.get()) {
+TargetOpts->Triple = "x86_64-apple-darwin11.1.0";
+Target = TargetInfo::CreateTargetInfo(Diags, TargetOpts);
+  }
+
+  void addSearchDir(llvm::StringRef Dir) {
+VFS->addFile(Dir, 0, llvm::MemoryBuffer::getMemBuffer(""), /*User=*/None,
+ /*Group=*/None, llvm::sys::fs::file_type::directory_file);
+const DirectoryEntry *DE = FileMgr.getDirectory(Dir);
+assert(DE);
+auto DL = DirectoryLookup(DE, SrcMgr::C_User, /*isFramework=*/false);
+Search.AddSearchPath(DL, /*isAngled=*/false);
+  }
+
+  IntrusiveRefCntPtr VFS;
+  FileSystemOptions FileMgrOpts;
+  FileManager FileMgr;
+  IntrusiveRefCntPtr DiagID;
+  DiagnosticsEngine Diags;
+  SourceManager SourceMgr;
+  LangOptions LangOpts;
+  std::shared_ptr TargetOpts;
+  IntrusiveRefCntPtr Target;
+  HeaderSearch Search;
+};
+
+TEST_F(HeaderSearchTest, NoSearchDir) {
+  EXPECT_EQ(Search.search_dir_size(), 0u);
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/x/y/z", /*WorkingDir=*/""),
+"/x/y/z");
+}
+
+TEST_F(HeaderSearchTest, SimpleShorten) {
+  addSearchDir("/x");
+  addSearchDir("/x/y");
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/x/y/z", /*WorkingDir=*/""),
+"z");
+  addSearchDir("/a/b/");
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/a/b/c", /*WorkingDir=*/""),
+"c");
+}
+
+TEST_F(HeaderSearchTest, ShortenWithWorkingDir) {
+  addSearchDir("x/y");
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/a/b/c/x/y/z",
+   /*WorkingDir=*/"/a/b/c"),
+"z");
+}
+
+TEST_F(HeaderSearchTest, Dots) {
+  addSearchDir("/x/./y/");
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/x/y/./z",
+   /*WorkingDir=*/""),
+"z");
+  addSearchDir("a/.././c/");
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/m/n/./c/z",
+   /*WorkingDir=*/"/m/n/"),
+"z");
+}
+
+} // namespace
+} // namespace clang
Index: unittests/Lex/CMakeLists.txt
===
--- unittests/Lex/CMakeLists.txt
+++ unittests/Lex/CMakeLists.txt
@@ -4,6 +4,7 @@
 
 add_clang_unittest(LexTests
   HeaderMapTest.cpp
+  HeaderSearchTest.cpp
   LexerTest.cpp
   PPCallbacksTest.cpp
   PPConditionalDirectiveRecordTest.cpp
Index: lib/Lex/HeaderSearch.cpp
===
--- lib/Lex/HeaderSearch.cpp
+++ lib/Lex/HeaderSearch.cpp
@@ -1580,9 +1580,15 @@
 std::string HeaderSearch::suggestPathToFileForDiagnostics(const FileEntry *File,
   bool *IsSystem) {
   // FIXME: We assume that the path name currently cached in the FileEntry is
-  // the most appropriate one for this analysis (and that it's spelled the same
-  // way as the corresponding header search path).
-  StringRef Name = File->getName();
+  // the most appropriate one for this analysis (and that it's spelled the
+  // same way as the corresponding header search path).
+  return suggestPathToFileForDiagnostics(File->getName(), /*BuildDir=*/"",
+ 

[PATCH] D42624: [clang-tidy] Add a utility Matcher to match the next statement within a statement sequence

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/utils/Matchers.h:16
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Analysis/CFG.h"
 

This will require linking in the clangAnalysis library as well; are we sure we 
want to take on this dependency here for all matchers?



Comment at: clang-tidy/utils/Matchers.h:58-60
+  // We get the first parent, making sure that we're not in a case statement
+  // not in a compound statement directly inside a switch, because this causes
+  // the buildCFG call to crash.

Why does it cause the crash? Should that crash not be fixed instead of applying 
this workaround?



Comment at: clang-tidy/utils/Matchers.h:70
+
+  // We build a Control Flow Graph (CFG) from the parent statement.
+  std::unique_ptr StatementCFG =

No need for the parenthetical. It's generally understood what a CFG is, so you 
can just use the acronym.



Comment at: clang-tidy/utils/Matchers.h:74-75
+  CFG::BuildOptions());
+
+  if (!StatementCFG) {
+return false;

Elide braces (here and elsewhere).



Comment at: clang-tidy/utils/Matchers.h:81
+  // all the possible branches of the code and therefore cover all statements.
+  for (auto& Block : *StatementCFG) {
+if (!Block) {

Formatting (here and elsewhere): you should run the patch through clang-format.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42624



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

[PATCH] D42640: [clangd] Prototype: collect symbol #include & insert #include in global code completion.

[Eric Liu via Phabricator via cfe-commits]

ioeric created this revision.
Herald added subscribers: cfe-commits, hintonda, jkorous-apple, ilya-biryukov, 
mgorny, klimek.

o Collect suitable #include paths for index symbols. This also does smart 
mapping
for STL symbols and IWYU pragma.
o For global code completion, add a command for inserting new #include in each 
code
completion item.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42640

Files:
  clangd/CMakeLists.txt
  clangd/ClangdLSPServer.cpp
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/CodeComplete.cpp
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/clients/clangd-vscode/package.json
  clangd/global-symbol-builder/CMakeLists.txt
  clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
  clangd/global-symbol-builder/PragmaCommentHandler.cpp
  clangd/global-symbol-builder/PragmaCommentHandler.h
  clangd/global-symbol-builder/STLPostfixHeaderMap.cpp
  clangd/global-symbol-builder/STLPostfixHeaderMap.h
  clangd/index/HeaderMapCollector.cpp
  clangd/index/HeaderMapCollector.h
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/Merge.cpp
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolCollector.h
  clangd/index/SymbolYAML.cpp
  clangd/tool/ClangdMain.cpp
  test/clangd/initialize-params-invalid.test
  test/clangd/initialize-params.test

Index: test/clangd/initialize-params.test
===
--- test/clangd/initialize-params.test
+++ test/clangd/initialize-params.test
@@ -28,7 +28,8 @@
 # CHECK-NEXT:  "documentRangeFormattingProvider": true,
 # CHECK-NEXT:  "executeCommandProvider": {
 # CHECK-NEXT:"commands": [
-# CHECK-NEXT:  "clangd.applyFix"
+# CHECK-NEXT:  "clangd.applyFix",
+# CHECK-NEXT:  "clangd.insertInclude"
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "renameProvider": true,
Index: test/clangd/initialize-params-invalid.test
===
--- test/clangd/initialize-params-invalid.test
+++ test/clangd/initialize-params-invalid.test
@@ -28,7 +28,8 @@
 # CHECK-NEXT:  "documentRangeFormattingProvider": true,
 # CHECK-NEXT:  "executeCommandProvider": {
 # CHECK-NEXT:"commands": [
-# CHECK-NEXT:  "clangd.applyFix"
+# CHECK-NEXT:  "clangd.applyFix",
+# CHECK-NEXT:  "clangd.insertInclude"
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "renameProvider": true,
Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -16,7 +16,9 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
+#include "llvm/Support/Signals.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
 #include 
 #include 
 #include 
Index: clangd/index/SymbolYAML.cpp
===
--- clangd/index/SymbolYAML.cpp
+++ clangd/index/SymbolYAML.cpp
@@ -64,6 +64,7 @@
   static void mapping(IO &io, Symbol::Details &Detail) {
 io.mapOptional("Documentation", Detail.Documentation);
 io.mapOptional("CompletionDetail", Detail.CompletionDetail);
+io.mapOptional("HeaderUri", Detail.HeaderUri);
   }
 };
 
Index: clangd/index/SymbolCollector.h
===
--- clangd/index/SymbolCollector.h
+++ clangd/index/SymbolCollector.h
@@ -17,6 +17,8 @@
 namespace clang {
 namespace clangd {
 
+class HeaderMapCollector;
+
 /// \brief Collect top-level symbols from an AST. These are symbols defined
 /// immediately inside a namespace or a translation unit scope. For example,
 /// symbols in classes or functions are not collected.
@@ -30,6 +32,10 @@
 /// Whether to collect symbols in main files (e.g. the source file
 /// corresponding to a TU).
 bool IndexMainFiles = false;
+bool CollectIncludePath = false;
+/// If set, this is used to map symbol #include path to a potentially
+/// different #include path.
+HeaderMapCollector *HeaderMap = nullptr;
   };
 
   SymbolCollector(Options Opts);
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -9,6 +9,8 @@
 
 #include "SymbolCollector.h"
 #include "../CodeCompletionStrings.h"
+#include "../URI.h"
+#include "HeaderMapCollector.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Basic/SourceManager.h"
@@ -105,6 +107,60 @@
   return false;
 }
 
+// We only collect #include paths for symbols that are suitable for global code
+// completion, except for namespaces since #include path for a namespace is hard
+// to define.
+bool shouldCollectIncludePath(index::SymbolKind Kind) {
+  using SK = index::SymbolKind;
+  switch (Kind) {
+  case SK::Macro:
+  case SK:

[PATCH] D42641: [MinGW] Emit typeinfo locally for dllimported classes without key functions

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added reviewers: rnk, majnemer, compnerd, smeenai.

This fixes building Qt as shared libraries with clang in MinGW mode; previously 
subclasses of the QObjectData class (in other DLLs than the base DLL) failed to 
find the typeinfo symbols (that neither were emitted in the base DLL or in the 
subclass component).

If the virtual destructor in the newly added testcase wouldn't be pure (or if 
there'd be another non-pure virtual method), it'd be a key function and things 
would work out as intended. Make sure to locally emit the typeinfo for those 
classes as well.

This matches what GCC does in this specific testcase (although it's unclear 
whether the generic mechanics are the same or different).

This fixes the root issue that spawned PR35146. The difference to GCC that is 
described in that bug still is present though.

I admit that this is a kinda hack-and-slash type of fix since I don't really 
understand how all these concepts fit together. Better suggestions on solving 
that particular issue are welcome.


Repository:
  rC Clang

https://reviews.llvm.org/D42641

Files:
  lib/CodeGen/CGVTables.cpp
  test/CodeGenCXX/dllimport-missing-key.cpp
  test/CodeGenCXX/dllimport-rtti.cpp
  test/CodeGenCXX/dllimport.cpp


Index: test/CodeGenCXX/dllimport.cpp
===
--- test/CodeGenCXX/dllimport.cpp
+++ test/CodeGenCXX/dllimport.cpp
@@ -658,7 +658,7 @@
 USECLASS(W)
 // vftable:
 // MO1-DAG: @"\01??_SW@@6B@" = linkonce_odr unnamed_addr constant { [1 x i8*] 
} { [1 x i8*] [i8* bitcast (void (%struct.W*)* @"\01?foo@W@@UAEXXZ" to i8*)] }
-// GO1-DAG: @_ZTV1W = available_externally dllimport unnamed_addr constant { 
[3 x i8*] } { [3 x i8*] [i8* null, i8* null, i8* bitcast (void (%struct.W*)* 
@_ZN1W3fooEv to i8*)] }
+// GO1-DAG: @_ZTV1W = external dllimport unnamed_addr constant { [3 x i8*] }
 
 struct __declspec(dllimport) KeyFuncClass {
   constexpr KeyFuncClass() {}
Index: test/CodeGenCXX/dllimport-rtti.cpp
===
--- test/CodeGenCXX/dllimport-rtti.cpp
+++ test/CodeGenCXX/dllimport-rtti.cpp
@@ -12,7 +12,7 @@
 // MSVC-DAG: @"\01??_R3S@@8" = linkonce_odr
 
 // GNU-DAG: @_ZTV1S = available_externally dllimport
-// GNU-DAG: @_ZTI1S = external dllimport
+// GNU-DAG: @_ZTI1S = linkonce_odr
 
 struct U : S {
 } u;
Index: test/CodeGenCXX/dllimport-missing-key.cpp
===
--- /dev/null
+++ test/CodeGenCXX/dllimport-missing-key.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -triple i686-windows-gnu -emit-llvm -std=c++1y -O0 -o - %s 
-w | FileCheck --check-prefix=GNU %s
+
+class __declspec(dllimport) QObjectData {
+public:
+virtual ~QObjectData() = 0;
+void *ptr;
+
+int method() const;
+};
+
+QObjectData::~QObjectData() {}
+
+int QObjectData::method() const
+{
+return 0;
+}
+
+class LocalClass : public QObjectData {
+};
+
+void call() {
+(new LocalClass())->method();
+}
+
+// GNU-DAG: @_ZTV11QObjectData = external dllimport
+// GNU-DAG: @_ZTS11QObjectData = linkonce_odr
+// GNU-DAG: @_ZTI11QObjectData = linkonce_odr
Index: lib/CodeGen/CGVTables.cpp
===
--- lib/CodeGen/CGVTables.cpp
+++ lib/CodeGen/CGVTables.cpp
@@ -884,6 +884,9 @@
   if (CGM.getTarget().getCXXABI().isMicrosoft())
 return false;
 
+  if (CGM.getTriple().isWindowsGNUEnvironment() && 
RD->hasAttr())
+return true;
+
   // If we have an explicit instantiation declaration (and not a
   // definition), the vtable is defined elsewhere.
   TemplateSpecializationKind TSK = RD->getTemplateSpecializationKind();


Index: test/CodeGenCXX/dllimport.cpp
===
--- test/CodeGenCXX/dllimport.cpp
+++ test/CodeGenCXX/dllimport.cpp
@@ -658,7 +658,7 @@
 USECLASS(W)
 // vftable:
 // MO1-DAG: @"\01??_SW@@6B@" = linkonce_odr unnamed_addr constant { [1 x i8*] } { [1 x i8*] [i8* bitcast (void (%struct.W*)* @"\01?foo@W@@UAEXXZ" to i8*)] }
-// GO1-DAG: @_ZTV1W = available_externally dllimport unnamed_addr constant { [3 x i8*] } { [3 x i8*] [i8* null, i8* null, i8* bitcast (void (%struct.W*)* @_ZN1W3fooEv to i8*)] }
+// GO1-DAG: @_ZTV1W = external dllimport unnamed_addr constant { [3 x i8*] }
 
 struct __declspec(dllimport) KeyFuncClass {
   constexpr KeyFuncClass() {}
Index: test/CodeGenCXX/dllimport-rtti.cpp
===
--- test/CodeGenCXX/dllimport-rtti.cpp
+++ test/CodeGenCXX/dllimport-rtti.cpp
@@ -12,7 +12,7 @@
 // MSVC-DAG: @"\01??_R3S@@8" = linkonce_odr
 
 // GNU-DAG: @_ZTV1S = available_externally dllimport
-// GNU-DAG: @_ZTI1S = external dllimport
+// GNU-DAG: @_ZTI1S = linkonce_odr
 
 struct U : S {
 } u;
Index: test/CodeGenCXX/dllimport-missing-key.cpp
===
--- /dev/null
+++ test/CodeGenCXX/dllimpor

[PATCH] D42641: [MinGW] Emit typeinfo locally for dllimported classes without key functions

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo updated this revision to Diff 131783.
mstorsjo added a comment.

Remove a few accidentally leftover parts of the new testcase.


https://reviews.llvm.org/D42641

Files:
  lib/CodeGen/CGVTables.cpp
  test/CodeGenCXX/dllimport-missing-key.cpp
  test/CodeGenCXX/dllimport-rtti.cpp
  test/CodeGenCXX/dllimport.cpp


Index: test/CodeGenCXX/dllimport.cpp
===
--- test/CodeGenCXX/dllimport.cpp
+++ test/CodeGenCXX/dllimport.cpp
@@ -658,7 +658,7 @@
 USECLASS(W)
 // vftable:
 // MO1-DAG: @"\01??_SW@@6B@" = linkonce_odr unnamed_addr constant { [1 x i8*] 
} { [1 x i8*] [i8* bitcast (void (%struct.W*)* @"\01?foo@W@@UAEXXZ" to i8*)] }
-// GO1-DAG: @_ZTV1W = available_externally dllimport unnamed_addr constant { 
[3 x i8*] } { [3 x i8*] [i8* null, i8* null, i8* bitcast (void (%struct.W*)* 
@_ZN1W3fooEv to i8*)] }
+// GO1-DAG: @_ZTV1W = external dllimport unnamed_addr constant { [3 x i8*] }
 
 struct __declspec(dllimport) KeyFuncClass {
   constexpr KeyFuncClass() {}
Index: test/CodeGenCXX/dllimport-rtti.cpp
===
--- test/CodeGenCXX/dllimport-rtti.cpp
+++ test/CodeGenCXX/dllimport-rtti.cpp
@@ -12,7 +12,7 @@
 // MSVC-DAG: @"\01??_R3S@@8" = linkonce_odr
 
 // GNU-DAG: @_ZTV1S = available_externally dllimport
-// GNU-DAG: @_ZTI1S = external dllimport
+// GNU-DAG: @_ZTI1S = linkonce_odr
 
 struct U : S {
 } u;
Index: test/CodeGenCXX/dllimport-missing-key.cpp
===
--- /dev/null
+++ test/CodeGenCXX/dllimport-missing-key.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple i686-windows-gnu -emit-llvm -std=c++1y -O0 -o - %s 
-w | FileCheck --check-prefix=GNU %s
+
+class __declspec(dllimport) QObjectData {
+public:
+virtual ~QObjectData() = 0;
+void *ptr;
+
+int method() const;
+};
+
+class LocalClass : public QObjectData {
+};
+
+void call() {
+(new LocalClass())->method();
+}
+
+// GNU-DAG: @_ZTV11QObjectData = external dllimport
+// GNU-DAG: @_ZTS11QObjectData = linkonce_odr
+// GNU-DAG: @_ZTI11QObjectData = linkonce_odr
Index: lib/CodeGen/CGVTables.cpp
===
--- lib/CodeGen/CGVTables.cpp
+++ lib/CodeGen/CGVTables.cpp
@@ -884,6 +884,9 @@
   if (CGM.getTarget().getCXXABI().isMicrosoft())
 return false;
 
+  if (CGM.getTriple().isWindowsGNUEnvironment() && 
RD->hasAttr())
+return true;
+
   // If we have an explicit instantiation declaration (and not a
   // definition), the vtable is defined elsewhere.
   TemplateSpecializationKind TSK = RD->getTemplateSpecializationKind();


Index: test/CodeGenCXX/dllimport.cpp
===
--- test/CodeGenCXX/dllimport.cpp
+++ test/CodeGenCXX/dllimport.cpp
@@ -658,7 +658,7 @@
 USECLASS(W)
 // vftable:
 // MO1-DAG: @"\01??_SW@@6B@" = linkonce_odr unnamed_addr constant { [1 x i8*] } { [1 x i8*] [i8* bitcast (void (%struct.W*)* @"\01?foo@W@@UAEXXZ" to i8*)] }
-// GO1-DAG: @_ZTV1W = available_externally dllimport unnamed_addr constant { [3 x i8*] } { [3 x i8*] [i8* null, i8* null, i8* bitcast (void (%struct.W*)* @_ZN1W3fooEv to i8*)] }
+// GO1-DAG: @_ZTV1W = external dllimport unnamed_addr constant { [3 x i8*] }
 
 struct __declspec(dllimport) KeyFuncClass {
   constexpr KeyFuncClass() {}
Index: test/CodeGenCXX/dllimport-rtti.cpp
===
--- test/CodeGenCXX/dllimport-rtti.cpp
+++ test/CodeGenCXX/dllimport-rtti.cpp
@@ -12,7 +12,7 @@
 // MSVC-DAG: @"\01??_R3S@@8" = linkonce_odr
 
 // GNU-DAG: @_ZTV1S = available_externally dllimport
-// GNU-DAG: @_ZTI1S = external dllimport
+// GNU-DAG: @_ZTI1S = linkonce_odr
 
 struct U : S {
 } u;
Index: test/CodeGenCXX/dllimport-missing-key.cpp
===
--- /dev/null
+++ test/CodeGenCXX/dllimport-missing-key.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple i686-windows-gnu -emit-llvm -std=c++1y -O0 -o - %s -w | FileCheck --check-prefix=GNU %s
+
+class __declspec(dllimport) QObjectData {
+public:
+virtual ~QObjectData() = 0;
+void *ptr;
+
+int method() const;
+};
+
+class LocalClass : public QObjectData {
+};
+
+void call() {
+(new LocalClass())->method();
+}
+
+// GNU-DAG: @_ZTV11QObjectData = external dllimport
+// GNU-DAG: @_ZTS11QObjectData = linkonce_odr
+// GNU-DAG: @_ZTI11QObjectData = linkonce_odr
Index: lib/CodeGen/CGVTables.cpp
===
--- lib/CodeGen/CGVTables.cpp
+++ lib/CodeGen/CGVTables.cpp
@@ -884,6 +884,9 @@
   if (CGM.getTarget().getCXXABI().isMicrosoft())
 return false;
 
+  if (CGM.getTriple().isWindowsGNUEnvironment() && RD->hasAttr())
+return true;
+
   // If we have an explicit instantiation declaration (and not a
   // definition), the vtable is defined elsewhere.
   TemplateSpecializationKind TSK = RD->getTemplateS

[PATCH] D42638: [clangd] Add a fallback directory for collected symbols with relative paths.

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.



Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:41
+llvm::cl::desc(
+"For symbols' file paths that cannot be resolved to absolute "
+"paths (i.e. in-memory VFS without absolute paths), "

I have trouble understanding this.

"fallback dir" is maybe too vague a name for this flag - 
"assume-header-directory" or "header-base-dir" or something?

The description assumes some context about the problem - can we provide it?
Maybe:
  The index includes the header that a symbol is defined in.
  If the absolute path cannot be determined (e.g. an in-memory VFS) then
  the relative path is resolved against this directory, which must be absolute.
  If this flag is not given, such headers will have relative paths.



Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:42
+"For symbols' file paths that cannot be resolved to absolute "
+"paths (i.e. in-memory VFS without absolute paths), "
+"combine the fallback directory path with file paths to get absolute "

nit: i.e -> e.g



Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:113
+  !llvm::sys::path::is_absolute(clang::clangd::FallbackDir)) {
+llvm::errs() << "--prefix-dir must be an absolute path.\n";
+return 1;

wrong flag name :-)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42638



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

[PATCH] D42642: [CUDA] Detect installation in PATH

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld created this revision.
Hahnfeld added reviewers: jlebar, tra.
Herald added a subscriber: cfe-commits.

If the CUDA toolkit is not installed to its default locations
in /usr/local/cuda, the user is forced to specify --cuda-path.
This is tedious and the driver can be smarter if well-known tools
(like ptxas) can already be found in the PATH environment variable.

Add option --cuda-path-ignore-env if the user wants to ignore
set environment variables. Also use it in the tests to make sure
the driver always finds the same CUDA installation, regardless
of the user's environment.


Repository:
  rC Clang

https://reviews.llvm.org/D42642

Files:
  include/clang/Driver/Options.td
  lib/Driver/ToolChains/Cuda.cpp
  test/Driver/Inputs/CUDA/usr/local/cuda/bin/ptxas
  test/Driver/cuda-detect-path.cu
  test/Driver/cuda-detect.cu
  test/Driver/cuda-not-found.cu
  test/Driver/cuda-version-check.cu

Index: test/Driver/cuda-version-check.cu
===
--- test/Driver/cuda-version-check.cu
+++ test/Driver/cuda-version-check.cu
@@ -2,50 +2,50 @@
 // REQUIRES: x86-registered-target
 // REQUIRES: nvptx-registered-target
 
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_20 --sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_20 --cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=OK
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_20 --sysroot=%S/Inputs/CUDA_80 2>&1 %s | \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_20 --cuda-path=%S/Inputs/CUDA_80/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=OK
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --sysroot=%S/Inputs/CUDA_80 2>&1 %s | \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-path=%S/Inputs/CUDA_80/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=OK
 
 // The installation at Inputs/CUDA is CUDA 7.0, which doesn't support sm_60.
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=ERR_SM60
 
 // This should only complain about sm_60, not sm_35.
 // RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-gpu-arch=sm_35 \
-// RUN:--sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN:--cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=ERR_SM60 --check-prefix=OK_SM35
 
 // We should get two errors here, one for sm_60 and one for sm_61.
 // RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-gpu-arch=sm_61 \
-// RUN:--sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN:--cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=ERR_SM60 --check-prefix=ERR_SM61
 
 // We should still get an error if we pass -nocudainc, because this compilation
 // would invoke ptxas, and we do a version check on that, too.
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 -nocudainc --sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 -nocudainc --cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=ERR_SM60
 
 // If with -nocudainc and -E, we don't touch the CUDA install, so we
 // shouldn't get an error.
 // RUN: %clang --target=x86_64-linux -v -### -E --cuda-device-only --cuda-gpu-arch=sm_60 -nocudainc \
-// RUN:--sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN:--cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=OK
 
 // --no-cuda-version-check should suppress all of these errors.
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --sysroot=%S/Inputs/CUDA 2>&1 \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 \
 // RUN:--no-cuda-version-check %s | \
 // RUN:FileCheck %s --check-prefix=OK
 
 // We need to make sure the version check is done only for the device toolchain,
 // therefore we should not get an error in host-only mode. We use the -S here
 // to avoid the error being produced in case by the assembler tool, which does
 // the same check.
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-host-only --sysroot=%S/Inputs/CUDA -S 2>&1 %s | \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-host-only --cuda-path=%S/Inputs/CUDA/usr/local/cuda -S 2>&1 %s | \
 // RUN:FileCheck %s --check-prefix=OK
-// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-device-only --sysroot=%S/Inputs/CUDA -S 2>&1 %s | \
+// RUN: %clang --target=x86_64-linux -v -### --cuda-gpu-arch=sm_60 --cuda-d

[clang-tools-extra] r323652 - [clangd] Fixed null deference on invalid compile commands.

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Mon Jan 29 06:30:28 2018
New Revision: 323652

URL: http://llvm.org/viewvc/llvm-project?rev=323652&view=rev
Log:
[clangd] Fixed null deference on invalid compile commands.

Code building the AST expected that construction of CompilerInstance
always succeeds. This is not the case.

Modified:
clang-tools-extra/trunk/clangd/ClangdUnit.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=323652&r1=323651&r2=323652&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Mon Jan 29 06:30:28 2018
@@ -244,6 +244,8 @@ ParsedAST::Build(const Context &Ctx,
   auto Clang = prepareCompilerInstance(
   std::move(CI), PreamblePCH, std::move(Buffer), std::move(PCHs),
   std::move(VFS), /*ref*/ UnitDiagsConsumer);
+  if (!Clang)
+return llvm::None;
 
   // Recover resources if we crash before exiting this method.
   llvm::CrashRecoveryContextCleanupRegistrar CICleanup(


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

[PATCH] D42644: [asan] Intercept std::rethrow_exception indirectly.

[Robert Schneider via Phabricator via cfe-commits]

robot created this revision.
robot added a reviewer: cryptoad.
robot added a project: Sanitizers.
Herald added subscribers: Sanitizers, llvm-commits, hintonda, mgorny, 
kubamracek.

Fixes Bug 32434
See https://bugs.llvm.org/show_bug.cgi?id=32434

Short summary:
std::rethrow_exception does not use __cxa_throw to rethrow the exception, so if
it is called from uninstrumented code, it will leave the stack poisoned. This
can lead to false positives.

Long description:

For functions which don't return normally (e.g. via exceptions), asan needs to
unpoison the entire stack. It is not known before a call to such a function
where execution will continue, some function which don't contain cleanup code
like destructors might be skipped. After stack unwinding, execution might
continue in uninstrumented code.

If the stack has been poisoned before such a function is called, but the stack
is unwound during the unconventional return, then zombie redzones (entries) for
no longer existing stack variables can remain in the shadow memory. Normally,
this is avoided by asan generating a call to __asan_handle_no_return before all
functions marked as [[noreturn]]. This __asan_handle_no_return unpoisons the
entire stack. Since these [[noreturn]] functions can be called from
uninstrumented code, asan also introduces interceptor functions which call
__asan_handle_no_return before running the original [[noreturn]] function;
for example, __cxa_throw is intercepted.

If a [[noreturn]] function is called from uninstrumented code (so the stack is
left poisoned) and additionally, execution continues in uninstrumented code, new
stack variables might be introduced and overlap with the stack variables
which have been removed during stack unwinding. Since the redzones are not
cleared nor overwritten by uninstrumented code, they remain but now contain
invalid data.

Now, if the redzones are checked against the new stack variables, false
positive reports can occur. This can happen for example by the uninstrumented
code calling an intercepted function such as memcpy, or an instrumented
function.

Intercepting std::rethrow_exception directly is not easily possible since it
depends on the C++ standard library implementation (e.g. libcxx vs libstdc++)
and the mangled name it produces for this function. As a rather simple
workaround, we're intercepting _Unwind_RaiseException for libstdc++. For
libcxxabi, we can intercept the ABI function __cxa_rethrow_primary_exception.


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D42644

Files:
  lib/asan/asan_interceptors.cc
  lib/asan/asan_interceptors.h
  lib/asan/tests/CMakeLists.txt
  lib/asan/tests/asan_rethrow_exception_test.cc

Index: lib/asan/tests/asan_rethrow_exception_test.cc
===
--- /dev/null
+++ lib/asan/tests/asan_rethrow_exception_test.cc
@@ -0,0 +1,94 @@
+#if ASAN_HAS_EXCEPTIONS
+
+
+#include "asan_test_utils.h"
+
+#include 
+#include 
+
+
+namespace {
+
+
+// Not instrumented because std::rethrow_exception is a [[noreturn]] function, for which the
+// compiler would emit a call to __asan_handle_no_return which unpoisons the stack.
+// We emulate here some code not compiled with asan. This function is not [[noreturn]] because the
+// scenario we're emulating doesn't always throw. If it were [[noreturn]], the calling code would
+// emit a call to __asan_handle_no_return.
+void NOINLINE ATTRIBUTE_NO_SANITIZE_ADDRESS
+uninstrumented_rethrow_exception(std::exception_ptr const& exc_ptr)
+{
+  // if we just call rethrow_exception, the compiler knows this function is noreturn, even
+  // though it's not inlined
+  if(Ident(true))
+std::rethrow_exception(exc_ptr);
+}
+
+// access stack, instrumented by asan
+// Asan checks pass if either the stack is entirely unpoisoned around dst and src, or redzones
+// exist but don't overlap with src nor dst.
+// We want this function to emit asan checks (checking the shadow memory). This works either by
+// calling std::memcpy which is intercepted by asan, or by generating instrumented code (if memcpy
+// is resolved via a builtin). To make sure it works in either case, we place the memcpy outside of
+// uninstrumented_fill_and_use_stack and into this separate function, which is instrumented.
+void NOINLINE
+instrumented_use_stack(void* dst, void const* src, int size)
+{
+  std::memcpy(dst, src, size);
+}
+
+// Create variables on the stack without touching the shadow memory, and use them to check if there
+// are redzones remaining (from prior stack operations) on the stack.
+void ATTRIBUTE_NO_SANITIZE_ADDRESS
+uninstrumented_fill_and_use_stack()
+{
+  // memcpy is intercepted by asan which performs checks on src and dst
+  using T = int[1000];
+  T x{};
+  T y{};
+  instrumented_use_stack(&x, &y, sizeof(T));
+}
+
+
+// Create redzones for stack variables in shadow memory and call std::rethrow_exception which
+// should unpoison the entire stack.
+void NOINLINE
+create_redzones_a

[PATCH] D42419: [clangd] Use new URI with scheme support in place of the existing LSP URI

[Sam McCall via Phabricator via cfe-commits]

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

Can you also remove the URI-encoding hack from the VSCode client?




Comment at: clangd/Protocol.cpp:35
+}
+auto Resolved = URI::resolve(*U);
+if (!Resolved) {

I think you can just check that the scheme is file and pull out the path?
we don't want to expose custom URI schemes in LSP, I think


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42419



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

[PATCH] D42638: [clangd] Add a fallback directory for collected symbols with relative paths.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 131799.
ioeric marked 3 inline comments as done.
ioeric added a comment.

- Addressed review comments.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42638

Files:
  clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolCollector.h
  unittests/clangd/SymbolCollectorTests.cpp

Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -44,6 +44,7 @@
   return arg.CompletionSnippetInsertText == S;
 }
 MATCHER_P(QName, Name, "") { return (arg.Scope + arg.Name).str() == Name; }
+MATCHER_P(Path, P, "") { return arg.CanonicalDeclaration.FilePath == P; }
 
 namespace clang {
 namespace clangd {
@@ -145,18 +146,25 @@
   runSymbolCollector(Header, Main);
   EXPECT_THAT(Symbols,
   UnorderedElementsAreArray(
-  {QName("Foo"),
-   QName("f1"),
-   QName("f2"),
-   QName("KInt"),
-   QName("kStr"),
-   QName("foo"),
-   QName("foo::bar"),
-   QName("foo::int32"),
-   QName("foo::int32_t"),
-   QName("foo::v1"),
-   QName("foo::bar::v2"),
-   QName("foo::baz")}));
+  {QName("Foo"), QName("f1"), QName("f2"), QName("KInt"),
+   QName("kStr"), QName("foo"), QName("foo::bar"),
+   QName("foo::int32"), QName("foo::int32_t"), QName("foo::v1"),
+   QName("foo::bar::v2"), QName("foo::baz")}));
+}
+
+TEST_F(SymbolCollectorTest, SymbolRelativeNoFallback) {
+  CollectorOpts.IndexMainFiles = false;
+  runSymbolCollector("class Foo {};", /*Main=*/"");
+  EXPECT_THAT(Symbols,
+  UnorderedElementsAre(AllOf(QName("Foo"), Path("symbols.h";
+}
+
+TEST_F(SymbolCollectorTest, SymbolRelativeWithFallback) {
+  CollectorOpts.IndexMainFiles = false;
+  CollectorOpts.FallbackDir = "/cwd";
+  runSymbolCollector("class Foo {};", /*Main=*/"");
+  EXPECT_THAT(Symbols, UnorderedElementsAre(
+   AllOf(QName("Foo"), Path("/cwd/symbols.h";
 }
 
 TEST_F(SymbolCollectorTest, IncludeEnums) {
Index: clangd/index/SymbolCollector.h
===
--- clangd/index/SymbolCollector.h
+++ clangd/index/SymbolCollector.h
@@ -30,6 +30,10 @@
 /// Whether to collect symbols in main files (e.g. the source file
 /// corresponding to a TU).
 bool IndexMainFiles = false;
+// When symbol paths cannot be resolved to absolute paths (e.g. files in
+// VFS that does not have absolute path), combine the fallback directory
+// with symbols' paths to get absolute paths. This must be an absolute path.
+std::string FallbackDir;
   };
 
   SymbolCollector(Options Opts);
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -14,44 +14,51 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Index/IndexSymbol.h"
 #include "clang/Index/USRGeneration.h"
+#include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 
 namespace clang {
 namespace clangd {
 
 namespace {
 // Make the Path absolute using the current working directory of the given
-// SourceManager if the Path is not an absolute path.
+// SourceManager if the Path is not an absolute path. If failed, this combine
+// relative paths with \p FallbackDir to get an absolute path.
 //
 // The Path can be a path relative to the build directory, or retrieved from
 // the SourceManager.
-std::string makeAbsolutePath(const SourceManager &SM, StringRef Path) {
+std::string makeAbsolutePath(const SourceManager &SM, StringRef Path,
+ StringRef FallbackDir) {
   llvm::SmallString<128> AbsolutePath(Path);
   if (std::error_code EC =
   SM.getFileManager().getVirtualFileSystem()->makeAbsolute(
   AbsolutePath))
 llvm::errs() << "Warning: could not make absolute file: '" << EC.message()
  << '\n';
-  // Handle the symbolic link path case where the current working directory
-  // (getCurrentWorkingDirectory) is a symlink./ We always want to the real
-  // file path (instead of the symlink path) for the  C++ symbols.
-  //
-  // Consider the following example:
-  //
-  //   src dir: /project/src/foo.h
-  //   current working directory (symlink): /tmp/build -> /project/src/
-  //
-  // The file path of Symbol is "/project/src/foo.h" instead of
-  // "/tmp/build/foo.h"
-  const DirectoryEntry *Dir = SM.getFileManager().getDirectory(
-  llvm::sys::path::parent_path(AbsolutePath.str()));
-  if (Dir) {
-StringRef DirName = SM.getFileManager(

[PATCH] D42638: [clangd] Add a fallback directory for collected symbols with relative paths.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:41
+llvm::cl::desc(
+"For symbols' file paths that cannot be resolved to absolute "
+"paths (i.e. in-memory VFS without absolute paths), "

sammccall wrote:
> I have trouble understanding this.
> 
> "fallback dir" is maybe too vague a name for this flag - 
> "assume-header-directory" or "header-base-dir" or something?
> 
> The description assumes some context about the problem - can we provide it?
> Maybe:
>   The index includes the header that a symbol is defined in.
>   If the absolute path cannot be determined (e.g. an in-memory VFS) then
>   the relative path is resolved against this directory, which must be 
> absolute.
>   If this flag is not given, such headers will have relative paths.
Thanks for the suggestion! Let's go with `--assume-header-dir`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42638



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

[clang-tools-extra] r323658 - [clangd] Add a fallback directory for collected symbols with relative paths.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Mon Jan 29 07:13:29 2018
New Revision: 323658

URL: http://llvm.org/viewvc/llvm-project?rev=323658&view=rev
Log:
[clangd] Add a fallback directory for collected symbols with relative paths.

Reviewers: hokein, sammccall

Subscribers: klimek, ilya-biryukov, jkorous-apple, cfe-commits

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

Modified:

clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/clangd/index/SymbolCollector.h
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp

Modified: 
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp?rev=323658&r1=323657&r2=323658&view=diff
==
--- 
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
 (original)
+++ 
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
 Mon Jan 29 07:13:29 2018
@@ -35,6 +35,14 @@ using clang::clangd::SymbolSlab;
 namespace clang {
 namespace clangd {
 
+static llvm::cl::opt AssumedHeaderDir(
+"assume-header-dir",
+llvm::cl::desc("The index includes header that a symbol is defined in. "
+   "If the absolute path cannot be determined (e.g. an "
+   "in-memory VFS) then the relative path is resolved against "
+   "this directory, which must be absolute. If this flag is "
+   "not given, such headers will have relative paths."));
+
 class SymbolIndexActionFactory : public tooling::FrontendActionFactory {
 public:
   SymbolIndexActionFactory(tooling::ExecutionContext *Ctx) : Ctx(Ctx) {}
@@ -72,9 +80,11 @@ public:
 IndexOpts.SystemSymbolFilter =
 index::IndexingOptions::SystemSymbolFilterKind::All;
 IndexOpts.IndexFunctionLocals = false;
+auto CollectorOpts = SymbolCollector::Options();
+CollectorOpts.FallbackDir = AssumedHeaderDir;
 return new WrappedIndexAction(
-std::make_shared(SymbolCollector::Options()),
-IndexOpts, Ctx);
+std::make_shared(std::move(CollectorOpts)), IndexOpts,
+Ctx);
   }
 
   tooling::ExecutionContext *Ctx;
@@ -98,6 +108,12 @@ int main(int argc, const char **argv) {
 return 1;
   }
 
+  if (!clang::clangd::AssumedHeaderDir.empty() &&
+  !llvm::sys::path::is_absolute(clang::clangd::AssumedHeaderDir)) {
+llvm::errs() << "--assume-header-dir must be an absolute path.\n";
+return 1;
+  }
+
   auto Err = Executor->get()->execute(
   llvm::make_unique(
   Executor->get()->getExecutionContext()));

Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=323658&r1=323657&r2=323658&view=diff
==
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Mon Jan 29 
07:13:29 2018
@@ -14,6 +14,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Index/IndexSymbol.h"
 #include "clang/Index/USRGeneration.h"
+#include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 
@@ -22,36 +23,42 @@ namespace clangd {
 
 namespace {
 // Make the Path absolute using the current working directory of the given
-// SourceManager if the Path is not an absolute path.
+// SourceManager if the Path is not an absolute path. If failed, this combine
+// relative paths with \p FallbackDir to get an absolute path.
 //
 // The Path can be a path relative to the build directory, or retrieved from
 // the SourceManager.
-std::string makeAbsolutePath(const SourceManager &SM, StringRef Path) {
+std::string makeAbsolutePath(const SourceManager &SM, StringRef Path,
+ StringRef FallbackDir) {
   llvm::SmallString<128> AbsolutePath(Path);
   if (std::error_code EC =
   SM.getFileManager().getVirtualFileSystem()->makeAbsolute(
   AbsolutePath))
 llvm::errs() << "Warning: could not make absolute file: '" << EC.message()
  << '\n';
-  // Handle the symbolic link path case where the current working directory
-  // (getCurrentWorkingDirectory) is a symlink./ We always want to the real
-  // file path (instead of the symlink path) for the  C++ symbols.
-  //
-  // Consider the following example:
-  //
-  //   src dir: /project/src/foo.h
-  //   current working directory (symlink): /tmp/build -> /project/src/
-  //
-  // The file path of Symbol is "/project/src/foo.h" instead of
-  // "/tmp/build/foo.h"
-  const DirectoryEntry *Dir = SM.getFileManager().getDirectory(
-  llvm::sys::pat

[PATCH] D42638: [clangd] Add a fallback directory for collected symbols with relative paths.

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE323658: [clangd] Add a fallback directory for collected 
symbols with relative paths. (authored by ioeric, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D42638?vs=131799&id=131800#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42638

Files:
  clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolCollector.h
  unittests/clangd/SymbolCollectorTests.cpp

Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -44,6 +44,7 @@
   return arg.CompletionSnippetInsertText == S;
 }
 MATCHER_P(QName, Name, "") { return (arg.Scope + arg.Name).str() == Name; }
+MATCHER_P(Path, P, "") { return arg.CanonicalDeclaration.FilePath == P; }
 
 namespace clang {
 namespace clangd {
@@ -145,18 +146,25 @@
   runSymbolCollector(Header, Main);
   EXPECT_THAT(Symbols,
   UnorderedElementsAreArray(
-  {QName("Foo"),
-   QName("f1"),
-   QName("f2"),
-   QName("KInt"),
-   QName("kStr"),
-   QName("foo"),
-   QName("foo::bar"),
-   QName("foo::int32"),
-   QName("foo::int32_t"),
-   QName("foo::v1"),
-   QName("foo::bar::v2"),
-   QName("foo::baz")}));
+  {QName("Foo"), QName("f1"), QName("f2"), QName("KInt"),
+   QName("kStr"), QName("foo"), QName("foo::bar"),
+   QName("foo::int32"), QName("foo::int32_t"), QName("foo::v1"),
+   QName("foo::bar::v2"), QName("foo::baz")}));
+}
+
+TEST_F(SymbolCollectorTest, SymbolRelativeNoFallback) {
+  CollectorOpts.IndexMainFiles = false;
+  runSymbolCollector("class Foo {};", /*Main=*/"");
+  EXPECT_THAT(Symbols,
+  UnorderedElementsAre(AllOf(QName("Foo"), Path("symbols.h";
+}
+
+TEST_F(SymbolCollectorTest, SymbolRelativeWithFallback) {
+  CollectorOpts.IndexMainFiles = false;
+  CollectorOpts.FallbackDir = "/cwd";
+  runSymbolCollector("class Foo {};", /*Main=*/"");
+  EXPECT_THAT(Symbols, UnorderedElementsAre(
+   AllOf(QName("Foo"), Path("/cwd/symbols.h";
 }
 
 TEST_F(SymbolCollectorTest, IncludeEnums) {
Index: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
===
--- clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
+++ clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
@@ -35,6 +35,14 @@
 namespace clang {
 namespace clangd {
 
+static llvm::cl::opt AssumedHeaderDir(
+"assume-header-dir",
+llvm::cl::desc("The index includes header that a symbol is defined in. "
+   "If the absolute path cannot be determined (e.g. an "
+   "in-memory VFS) then the relative path is resolved against "
+   "this directory, which must be absolute. If this flag is "
+   "not given, such headers will have relative paths."));
+
 class SymbolIndexActionFactory : public tooling::FrontendActionFactory {
 public:
   SymbolIndexActionFactory(tooling::ExecutionContext *Ctx) : Ctx(Ctx) {}
@@ -72,9 +80,11 @@
 IndexOpts.SystemSymbolFilter =
 index::IndexingOptions::SystemSymbolFilterKind::All;
 IndexOpts.IndexFunctionLocals = false;
+auto CollectorOpts = SymbolCollector::Options();
+CollectorOpts.FallbackDir = AssumedHeaderDir;
 return new WrappedIndexAction(
-std::make_shared(SymbolCollector::Options()),
-IndexOpts, Ctx);
+std::make_shared(std::move(CollectorOpts)), IndexOpts,
+Ctx);
   }
 
   tooling::ExecutionContext *Ctx;
@@ -98,6 +108,12 @@
 return 1;
   }
 
+  if (!clang::clangd::AssumedHeaderDir.empty() &&
+  !llvm::sys::path::is_absolute(clang::clangd::AssumedHeaderDir)) {
+llvm::errs() << "--assume-header-dir must be an absolute path.\n";
+return 1;
+  }
+
   auto Err = Executor->get()->execute(
   llvm::make_unique(
   Executor->get()->getExecutionContext()));
Index: clangd/index/SymbolCollector.h
===
--- clangd/index/SymbolCollector.h
+++ clangd/index/SymbolCollector.h
@@ -30,6 +30,10 @@
 /// Whether to collect symbols in main files (e.g. the source file
 /// corresponding to a TU).
 bool IndexMainFiles = false;
+// When symbol paths cannot be resolved to absolute paths (e.g. files in
+// VFS that does not have absolute path), combine the fallback directory
+// with symbols' paths to get absolute paths. This must be an absolute path.
+std::string FallbackDir;
   };
 
   SymbolCollector(Options Opts);
Index: clangd/index/SymbolColle

[PATCH] D42638: [clangd] Add a fallback directory for collected symbols with relative paths.

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL323658: [clangd] Add a fallback directory for collected 
symbols with relative paths. (authored by ioeric, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D42638

Files:
  
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
  clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
  clang-tools-extra/trunk/clangd/index/SymbolCollector.h
  clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp

Index: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
@@ -44,6 +44,7 @@
   return arg.CompletionSnippetInsertText == S;
 }
 MATCHER_P(QName, Name, "") { return (arg.Scope + arg.Name).str() == Name; }
+MATCHER_P(Path, P, "") { return arg.CanonicalDeclaration.FilePath == P; }
 
 namespace clang {
 namespace clangd {
@@ -145,18 +146,25 @@
   runSymbolCollector(Header, Main);
   EXPECT_THAT(Symbols,
   UnorderedElementsAreArray(
-  {QName("Foo"),
-   QName("f1"),
-   QName("f2"),
-   QName("KInt"),
-   QName("kStr"),
-   QName("foo"),
-   QName("foo::bar"),
-   QName("foo::int32"),
-   QName("foo::int32_t"),
-   QName("foo::v1"),
-   QName("foo::bar::v2"),
-   QName("foo::baz")}));
+  {QName("Foo"), QName("f1"), QName("f2"), QName("KInt"),
+   QName("kStr"), QName("foo"), QName("foo::bar"),
+   QName("foo::int32"), QName("foo::int32_t"), QName("foo::v1"),
+   QName("foo::bar::v2"), QName("foo::baz")}));
+}
+
+TEST_F(SymbolCollectorTest, SymbolRelativeNoFallback) {
+  CollectorOpts.IndexMainFiles = false;
+  runSymbolCollector("class Foo {};", /*Main=*/"");
+  EXPECT_THAT(Symbols,
+  UnorderedElementsAre(AllOf(QName("Foo"), Path("symbols.h";
+}
+
+TEST_F(SymbolCollectorTest, SymbolRelativeWithFallback) {
+  CollectorOpts.IndexMainFiles = false;
+  CollectorOpts.FallbackDir = "/cwd";
+  runSymbolCollector("class Foo {};", /*Main=*/"");
+  EXPECT_THAT(Symbols, UnorderedElementsAre(
+   AllOf(QName("Foo"), Path("/cwd/symbols.h";
 }
 
 TEST_F(SymbolCollectorTest, IncludeEnums) {
Index: clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
===
--- clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
+++ clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
@@ -35,6 +35,14 @@
 namespace clang {
 namespace clangd {
 
+static llvm::cl::opt AssumedHeaderDir(
+"assume-header-dir",
+llvm::cl::desc("The index includes header that a symbol is defined in. "
+   "If the absolute path cannot be determined (e.g. an "
+   "in-memory VFS) then the relative path is resolved against "
+   "this directory, which must be absolute. If this flag is "
+   "not given, such headers will have relative paths."));
+
 class SymbolIndexActionFactory : public tooling::FrontendActionFactory {
 public:
   SymbolIndexActionFactory(tooling::ExecutionContext *Ctx) : Ctx(Ctx) {}
@@ -72,9 +80,11 @@
 IndexOpts.SystemSymbolFilter =
 index::IndexingOptions::SystemSymbolFilterKind::All;
 IndexOpts.IndexFunctionLocals = false;
+auto CollectorOpts = SymbolCollector::Options();
+CollectorOpts.FallbackDir = AssumedHeaderDir;
 return new WrappedIndexAction(
-std::make_shared(SymbolCollector::Options()),
-IndexOpts, Ctx);
+std::make_shared(std::move(CollectorOpts)), IndexOpts,
+Ctx);
   }
 
   tooling::ExecutionContext *Ctx;
@@ -98,6 +108,12 @@
 return 1;
   }
 
+  if (!clang::clangd::AssumedHeaderDir.empty() &&
+  !llvm::sys::path::is_absolute(clang::clangd::AssumedHeaderDir)) {
+llvm::errs() << "--assume-header-dir must be an absolute path.\n";
+return 1;
+  }
+
   auto Err = Executor->get()->execute(
   llvm::make_unique(
   Executor->get()->getExecutionContext()));
Index: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
@@ -14,44 +14,51 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Index/IndexSymbol.h"
 #include "clang/Index/USRGeneration.h"
+#include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/

[PATCH] D42419: [clangd] Use new URI with scheme support in place of the existing LSP URI

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 131802.
ioeric marked an inline comment as done.
ioeric added a comment.

- addressed review comments; removed URI hack in vscode extenstion.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42419

Files:
  clangd/ClangdLSPServer.cpp
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/URI.cpp
  clangd/URI.h
  clangd/XRefs.cpp
  clangd/clients/clangd-vscode/src/extension.ts
  unittests/clangd/URITests.cpp
  unittests/clangd/XRefsTests.cpp

Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -9,6 +9,7 @@
 #include "Annotations.h"
 #include "ClangdUnit.h"
 #include "Matchers.h"
+#include "TestFS.h"
 #include "XRefs.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/PCHContainerOperations.h"
@@ -38,7 +39,9 @@
 
 // FIXME: this is duplicated with FileIndexTests. Share it.
 ParsedAST build(StringRef Code) {
-  auto CI = createInvocationFromCommandLine({"clang", "-xc++", "Foo.cpp"});
+  auto TestFile = getVirtualTestFilePath("Foo.cpp");
+  auto CI =
+  createInvocationFromCommandLine({"clang", "-xc++", TestFile.c_str()});
   auto Buf = MemoryBuffer::getMemBuffer(Code);
   auto AST = ParsedAST::Build(
   Context::empty(), std::move(CI), nullptr, std::move(Buf),
Index: unittests/clangd/URITests.cpp
===
--- unittests/clangd/URITests.cpp
+++ unittests/clangd/URITests.cpp
@@ -40,13 +40,12 @@
 .str();
   }
 
-  llvm::Expected
+  llvm::Expected
   uriFromAbsolutePath(llvm::StringRef AbsolutePath) const override {
 auto Pos = AbsolutePath.find(TestRoot);
 assert(Pos != llvm::StringRef::npos);
-return FileURI::create(
-Scheme, /*Authority=*/"",
-AbsolutePath.substr(Pos + llvm::StringRef(TestRoot).size()));
+return URI(Scheme, /*Authority=*/"",
+   AbsolutePath.substr(Pos + llvm::StringRef(TestRoot).size()));
   }
 };
 
@@ -57,30 +56,22 @@
 
 std::string createOrDie(llvm::StringRef AbsolutePath,
 llvm::StringRef Scheme = "file") {
-  auto Uri = FileURI::create(AbsolutePath, Scheme);
+  auto Uri = URI::create(AbsolutePath, Scheme);
   if (!Uri)
 llvm_unreachable(llvm::toString(Uri.takeError()).c_str());
   return Uri->toString();
 }
 
-std::string createOrDie(llvm::StringRef Scheme, llvm::StringRef Authority,
-llvm::StringRef Body) {
-  auto Uri = FileURI::create(Scheme, Authority, Body);
-  if (!Uri)
-llvm_unreachable(llvm::toString(Uri.takeError()).c_str());
-  return Uri->toString();
-}
-
-FileURI parseOrDie(llvm::StringRef Uri) {
-  auto U = FileURI::parse(Uri);
+URI parseOrDie(llvm::StringRef Uri) {
+  auto U = URI::parse(Uri);
   if (!U)
 llvm_unreachable(llvm::toString(U.takeError()).c_str());
   return *U;
 }
 
 TEST(PercentEncodingTest, Encode) {
-  EXPECT_EQ(createOrDie("x", /*Authority=*/"", "a/b/c"), "x:a/b/c");
-  EXPECT_EQ(createOrDie("x", /*Authority=*/"", "a!b;c~"), "x:a%21b%3bc~");
+  EXPECT_EQ(URI("x", /*Authority=*/"", "a/b/c").toString(), "x:a/b/c");
+  EXPECT_EQ(URI("x", /*Authority=*/"", "a!b;c~").toString(), "x:a%21b%3bc~");
 }
 
 TEST(PercentEncodingTest, Decode) {
@@ -93,8 +84,8 @@
   EXPECT_EQ(parseOrDie("x:a%21b%3ac~").body(), "a!b:c~");
 }
 
-std::string resolveOrDie(const FileURI &U, llvm::StringRef HintPath = "") {
-  auto Path = FileURI::resolve(U, HintPath);
+std::string resolveOrDie(const URI &U, llvm::StringRef HintPath = "") {
+  auto Path = URI::resolve(U, HintPath);
   if (!Path)
 llvm_unreachable(llvm::toString(Path.takeError()).c_str());
   return *Path;
@@ -110,25 +101,16 @@
 }
 
 TEST(URITest, FailedCreate) {
-  auto Fail = [](llvm::Expected U) {
+  auto Fail = [](llvm::Expected U) {
 if (!U) {
   llvm::consumeError(U.takeError());
   return true;
 }
 return false;
   };
-  // Create from scheme+authority+body:
-  //
-  // Scheme must be provided.
-  EXPECT_TRUE(Fail(FileURI::create("", "auth", "/a")));
-  // Body must start with '/' if authority is present.
-  EXPECT_TRUE(Fail(FileURI::create("scheme", "auth", "x/y/z")));
-
-  // Create from scheme registry:
-  //
-  EXPECT_TRUE(Fail(FileURI::create("/x/y/z", "no")));
+  EXPECT_TRUE(Fail(URI::create("/x/y/z", "no")));
   // Path has to be absolute.
-  EXPECT_TRUE(Fail(FileURI::create("x/y/z")));
+  EXPECT_TRUE(Fail(URI::create("x/y/z", "file")));
 }
 
 TEST(URITest, Parse) {
@@ -163,7 +145,7 @@
 
 TEST(URITest, ParseFailed) {
   auto FailedParse = [](llvm::StringRef U) {
-auto URI = FileURI::parse(U);
+auto URI = URI::parse(U);
 if (!URI) {
   llvm::consumeError(URI.takeError());
   return true;
@@ -194,14 +176,14 @@
 
 TEST(URITest, Platform) {
   auto Path = getVirtualTestFilePath("x");
-  auto U = FileURI::create(Path, "file");
+  auto U = URI::create(Path, "file");
   EXPECT_TRUE(static_cast(U));
   

[PATCH] D42419: [clangd] Use new URI with scheme support in place of the existing LSP URI

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE323660: [clangd] Use new URI with scheme support in place 
of the existing LSP URI (authored by ioeric, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D42419?vs=131802&id=131803#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42419

Files:
  clangd/ClangdLSPServer.cpp
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/URI.cpp
  clangd/URI.h
  clangd/XRefs.cpp
  clangd/clients/clangd-vscode/src/extension.ts
  unittests/clangd/URITests.cpp
  unittests/clangd/XRefsTests.cpp

Index: unittests/clangd/URITests.cpp
===
--- unittests/clangd/URITests.cpp
+++ unittests/clangd/URITests.cpp
@@ -40,13 +40,12 @@
 .str();
   }
 
-  llvm::Expected
+  llvm::Expected
   uriFromAbsolutePath(llvm::StringRef AbsolutePath) const override {
 auto Pos = AbsolutePath.find(TestRoot);
 assert(Pos != llvm::StringRef::npos);
-return FileURI::create(
-Scheme, /*Authority=*/"",
-AbsolutePath.substr(Pos + llvm::StringRef(TestRoot).size()));
+return URI(Scheme, /*Authority=*/"",
+   AbsolutePath.substr(Pos + llvm::StringRef(TestRoot).size()));
   }
 };
 
@@ -57,30 +56,22 @@
 
 std::string createOrDie(llvm::StringRef AbsolutePath,
 llvm::StringRef Scheme = "file") {
-  auto Uri = FileURI::create(AbsolutePath, Scheme);
+  auto Uri = URI::create(AbsolutePath, Scheme);
   if (!Uri)
 llvm_unreachable(llvm::toString(Uri.takeError()).c_str());
   return Uri->toString();
 }
 
-std::string createOrDie(llvm::StringRef Scheme, llvm::StringRef Authority,
-llvm::StringRef Body) {
-  auto Uri = FileURI::create(Scheme, Authority, Body);
-  if (!Uri)
-llvm_unreachable(llvm::toString(Uri.takeError()).c_str());
-  return Uri->toString();
-}
-
-FileURI parseOrDie(llvm::StringRef Uri) {
-  auto U = FileURI::parse(Uri);
+URI parseOrDie(llvm::StringRef Uri) {
+  auto U = URI::parse(Uri);
   if (!U)
 llvm_unreachable(llvm::toString(U.takeError()).c_str());
   return *U;
 }
 
 TEST(PercentEncodingTest, Encode) {
-  EXPECT_EQ(createOrDie("x", /*Authority=*/"", "a/b/c"), "x:a/b/c");
-  EXPECT_EQ(createOrDie("x", /*Authority=*/"", "a!b;c~"), "x:a%21b%3bc~");
+  EXPECT_EQ(URI("x", /*Authority=*/"", "a/b/c").toString(), "x:a/b/c");
+  EXPECT_EQ(URI("x", /*Authority=*/"", "a!b;c~").toString(), "x:a%21b%3bc~");
 }
 
 TEST(PercentEncodingTest, Decode) {
@@ -93,8 +84,8 @@
   EXPECT_EQ(parseOrDie("x:a%21b%3ac~").body(), "a!b:c~");
 }
 
-std::string resolveOrDie(const FileURI &U, llvm::StringRef HintPath = "") {
-  auto Path = FileURI::resolve(U, HintPath);
+std::string resolveOrDie(const URI &U, llvm::StringRef HintPath = "") {
+  auto Path = URI::resolve(U, HintPath);
   if (!Path)
 llvm_unreachable(llvm::toString(Path.takeError()).c_str());
   return *Path;
@@ -110,25 +101,16 @@
 }
 
 TEST(URITest, FailedCreate) {
-  auto Fail = [](llvm::Expected U) {
+  auto Fail = [](llvm::Expected U) {
 if (!U) {
   llvm::consumeError(U.takeError());
   return true;
 }
 return false;
   };
-  // Create from scheme+authority+body:
-  //
-  // Scheme must be provided.
-  EXPECT_TRUE(Fail(FileURI::create("", "auth", "/a")));
-  // Body must start with '/' if authority is present.
-  EXPECT_TRUE(Fail(FileURI::create("scheme", "auth", "x/y/z")));
-
-  // Create from scheme registry:
-  //
-  EXPECT_TRUE(Fail(FileURI::create("/x/y/z", "no")));
+  EXPECT_TRUE(Fail(URI::create("/x/y/z", "no")));
   // Path has to be absolute.
-  EXPECT_TRUE(Fail(FileURI::create("x/y/z")));
+  EXPECT_TRUE(Fail(URI::create("x/y/z", "file")));
 }
 
 TEST(URITest, Parse) {
@@ -163,7 +145,7 @@
 
 TEST(URITest, ParseFailed) {
   auto FailedParse = [](llvm::StringRef U) {
-auto URI = FileURI::parse(U);
+auto URI = URI::parse(U);
 if (!URI) {
   llvm::consumeError(URI.takeError());
   return true;
@@ -194,14 +176,14 @@
 
 TEST(URITest, Platform) {
   auto Path = getVirtualTestFilePath("x");
-  auto U = FileURI::create(Path, "file");
+  auto U = URI::create(Path, "file");
   EXPECT_TRUE(static_cast(U));
   EXPECT_THAT(resolveOrDie(*U), Path.str());
 }
 
 TEST(URITest, ResolveFailed) {
   auto FailedResolve = [](llvm::StringRef Uri) {
-auto Path = FileURI::resolve(parseOrDie(Uri));
+auto Path = URI::resolve(parseOrDie(Uri));
 if (!Path) {
   llvm::consumeError(Path.takeError());
   return true;
Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -9,6 +9,7 @@
 #include "Annotations.h"
 #include "ClangdUnit.h"
 #include "Matchers.h"
+#include "TestFS.h"
 #include "XRefs.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/PCHContainerOperations.h"
@@ -38,7 +39,9 @@
 
 // FIXME: this is duplicated w

[clang-tools-extra] r323660 - [clangd] Use new URI with scheme support in place of the existing LSP URI

[Eric Liu via cfe-commits]

Author: ioeric
Date: Mon Jan 29 07:37:46 2018
New Revision: 323660

URL: http://llvm.org/viewvc/llvm-project?rev=323660&view=rev
Log:
[clangd] Use new URI with scheme support in place of the existing LSP URI

Summary:
o Replace the existing clangd::URI with a wrapper of FileURI which also
carries a resolved file path.
o s/FileURI/URI/
o Get rid of the URI hack in vscode extension.

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: klimek, ilya-biryukov, jkorous-apple, cfe-commits

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

Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/Protocol.cpp
clang-tools-extra/trunk/clangd/Protocol.h
clang-tools-extra/trunk/clangd/URI.cpp
clang-tools-extra/trunk/clangd/URI.h
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts
clang-tools-extra/trunk/unittests/clangd/URITests.cpp
clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=323660&r1=323659&r2=323660&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Mon Jan 29 07:37:46 2018
@@ -10,6 +10,7 @@
 #include "ClangdLSPServer.h"
 #include "JSONRPCDispatcher.h"
 #include "SourceCode.h"
+#include "URI.h"
 #include "llvm/Support/FormatVariadic.h"
 
 using namespace clang::clangd;
@@ -157,7 +158,7 @@ void ClangdLSPServer::onRename(Ctx C, Re
 
   std::vector Edits = replacementsToEdits(*Code, *Replacements);
   WorkspaceEdit WE;
-  WE.changes = {{Params.textDocument.uri.uri, Edits}};
+  WE.changes = {{Params.textDocument.uri.uri(), Edits}};
   reply(C, WE);
 }
 
@@ -227,7 +228,7 @@ void ClangdLSPServer::onCodeAction(Ctx C
 auto Edits = getFixIts(Params.textDocument.uri.file, D);
 if (!Edits.empty()) {
   WorkspaceEdit WE;
-  WE.changes = {{Params.textDocument.uri.uri, std::move(Edits)}};
+  WE.changes = {{Params.textDocument.uri.uri(), std::move(Edits)}};
   Commands.push_back(json::obj{
   {"title", llvm::formatv("Apply FixIt {0}", D.message)},
   {"command", ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND},
@@ -279,8 +280,7 @@ void ClangdLSPServer::onGoToDefinition(C
 void ClangdLSPServer::onSwitchSourceHeader(Ctx C,
TextDocumentIdentifier &Params) {
   llvm::Optional Result = Server.switchSourceHeader(Params.uri.file);
-  std::string ResultUri;
-  reply(C, Result ? URI::fromFile(*Result).uri : "");
+  reply(C, Result ? URI::createFile(*Result).toString() : "");
 }
 
 void ClangdLSPServer::onDocumentHighlight(Ctx C,
@@ -377,7 +377,7 @@ void ClangdLSPServer::onDiagnosticsReady
   {"method", "textDocument/publishDiagnostics"},
   {"params",
json::obj{
-   {"uri", URI::fromFile(File)},
+   {"uri", URIForFile{File}},
{"diagnostics", std::move(DiagnosticsJSON)},
}},
   });

Modified: clang-tools-extra/trunk/clangd/Protocol.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.cpp?rev=323660&r1=323659&r2=323660&view=diff
==
--- clang-tools-extra/trunk/clangd/Protocol.cpp (original)
+++ clang-tools-extra/trunk/clangd/Protocol.cpp Mon Jan 29 07:37:46 2018
@@ -12,6 +12,8 @@
 
//===--===//
 
 #include "Protocol.h"
+#include "URI.h"
+#include "Logger.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/Format.h"
@@ -22,45 +24,32 @@
 namespace clang {
 namespace clangd {
 
-URI URI::fromUri(llvm::StringRef uri) {
-  URI Result;
-  Result.uri = uri;
-  uri.consume_front("file://");
-  // Also trim authority-less URIs
-  uri.consume_front("file:");
-  // For Windows paths e.g. /X:
-  if (uri.size() > 2 && uri[0] == '/' && uri[2] == ':')
-uri.consume_front("/");
-  // Make sure that file paths are in native separators
-  Result.file = llvm::sys::path::convert_to_slash(uri);
-  return Result;
-}
-
-URI URI::fromFile(llvm::StringRef file) {
-  using namespace llvm::sys;
-  URI Result;
-  Result.file = file;
-  Result.uri = "file://";
-  // For Windows paths e.g. X:
-  if (file.size() > 1 && file[1] == ':')
-Result.uri += "/";
-  // Make sure that uri paths are with posix separators
-  Result.uri += path::convert_to_slash(file, path::Style::posix);
-  return Result;
-}
-
-bool fromJSON(const json::Expr &E, URI &R) {
+bool fromJSON(const json::Expr &E, URIForFile &R) {
   if (auto S = E.asString()) {
-R = URI::fromUri(*S);
+auto U = URI::parse(*S);
+if (!U) {
+  log(Context::empty(),
+  "Failed to parse UR

[PATCH] D42366: [CodeGen] Fix generation of TBAA tags for may-alias accesses

[Ivan Kosarev via Phabricator via cfe-commits]

kosarev added inline comments.



Comment at: lib/CodeGen/CodeGenTBAA.h:67
   /* BaseType= */ nullptr, /* AccessType= */ nullptr,
-  /* Offset= */ 0, /* Size= */ 0);
+  /* Offset= */ 0, /* Size= */ UINT64_MAX);
   }

rjmccall wrote:
> Hmm.  I think you should talk this part over with Hal.  If the size should be 
> ignored then I think it's better to just leave this as 0; it's certainly 
> easier to recognize 0 as a "size unknown" value.
Hal, in D41501 we use UINT64_MAX as the "unknown" size value. Should we do the 
same here?


Repository:
  rL LLVM

https://reviews.llvm.org/D42366



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

[PATCH] D42578: [AMDGPU] Add ds_fadd, ds_fmin, ds_fmax builtins functions

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: include/clang/Basic/BuiltinsAMDGPU.def:96-98
+BUILTIN(__builtin_amdgcn_ds_fadd, "ff*3fiib", "nc")
+BUILTIN(__builtin_amdgcn_ds_fmin, "ff*3fiib", "nc")
+BUILTIN(__builtin_amdgcn_ds_fmax, "ff*3fiib", "nc")

These are definitely not const intrinsics and need the c removed


Repository:
  rC Clang

https://reviews.llvm.org/D42578



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

[PATCH] D42645: New simple Checker for mmap calls

[David CARLIER via Phabricator via cfe-commits]

devnexen created this revision.
devnexen added reviewers: dcoughlin, dergachev.a.
devnexen created this object with visibility "All Users".
Herald added subscribers: cfe-commits, hintonda, mgorny.

This new checker tests if both PROT_WRITE and PROT_EXEC have been set.


Repository:
  rC Clang

https://reviews.llvm.org/D42645

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp

Index: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
@@ -0,0 +1,76 @@
+// MmapWriteExecChecker.cpp - Check for the prot argument
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This checker detects a common memory allocation security flaw.
+// Suppose 'unsigned int n' comes from an untrusted source. If the
+// code looks like 'malloc (n * 4)', and an attacker can make 'n' be
+// say MAX_UINT/4+2, then instead of allocating the correct 'n' 4-byte
+// elements, this will actually allocate only two because of overflow.
+// Then when the rest of the program attempts to store values past the
+// second element, these values will actually overwrite other items in
+// the heap, probably allowing the attacker to execute arbitrary code.
+//
+//===--===//
+
+#include "ClangSACheckers.h"
+
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+
+using namespace clang;
+using namespace ento;
+using llvm::APSInt;
+
+namespace {
+class MmapWriteExecChecker : public Checker> {
+#ifdef __GLIBC__
+  static const int ProtWrite = 0x02;
+  static const int ProtExec  = 0x01;
+#else
+  static const int ProtWrite = 0x02;
+  static const int ProtExec  = 0x04;
+#endif
+  mutable std::unique_ptr BT;
+public:
+  void checkPostStmt(const CallExpr *S, CheckerContext &C) const;
+};
+}
+
+void MmapWriteExecChecker::checkPostStmt(const CallExpr *CE,
+ CheckerContext &C) const {
+  StringRef FName = C.getCalleeName(CE);
+  if (FName.empty())
+return;
+
+  if (FName == "mmap") {
+SVal ProtVal = C.getSVal(CE->getArg(2)); 
+Optional Prot = ProtVal.getAs();
+int64_t prot = Prot->getValue().getSExtValue();
+
+if ((prot & (ProtWrite | ProtExec))) {
+  if (!BT) {
+BT.reset(new BugType(this, "W^X check fail", "Write Exec prot flags set"));
+ExplodedNode *N = C.generateErrorNode();
+if (!N)
+  return;
+
+auto Report = llvm::make_unique(
+ *BT, "Both PROT_WRITE and PROT_EXEC flags had been set", N);
+Report->addRange(CE->getArg(2)->getSourceRange());
+C.emitReport(std::move(Report));
+  }
+}
+  }
+}
+
+void ento::registerMmapWriteExecChecker(CheckerManager &mgr) {
+  mgr.registerChecker();
+}
Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt
===
--- lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -50,6 +50,7 @@
   MallocOverflowSecurityChecker.cpp
   MallocSizeofChecker.cpp
   MisusedMovedObjectChecker.cpp
+  MmapWriteExecChecker.cpp
   MPI-Checker/MPIBugReporter.cpp
   MPI-Checker/MPIChecker.cpp
   MPI-Checker/MPIFunctionClassifier.cpp
Index: include/clang/StaticAnalyzer/Checkers/Checkers.td
===
--- include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -86,7 +86,7 @@
 
 // The APIModeling package is for checkers that model APIs and don't perform
 // any diagnostics. These checkers are always turned on; this package is
-// intended for API modeling that is not controlled by the the target triple.
+// intended for API modeling that is not controlled by the target triple.
 def APIModeling : Package<"apiModeling">, Hidden;
 def GoogleAPIModeling : Package<"google">, InPackage;
 
@@ -414,6 +414,10 @@
   HelpText<"Check for overflows in the arguments to malloc()">,
   DescFile<"MallocOverflowSecurityChecker.cpp">;
 
+def MmapWriteExecChecker : Checker<"MmapWriteExecChecker">,
+  HelpText<"Check if mmap() call is not both writable and executable">,
+  DescFile<"MmapWriteExecChecker.cpp">;
+
 } // end "alpha.security"
 
 //===--===//
___
cfe-commits mailing l

Re: [clang-tools-extra] r323149 - [clangd] Drop ~destructor completions - rarely helpful and work inconsistently

[David Blaikie via cfe-commits]

Any chance of making this a really low priority completion? (maybe just
leaving in a FIXME or expected-fail check of some kind if it's not
practical to implement it right now) For those handful of times when you
actually want to do this.

On Mon, Jan 22, 2018 at 1:06 PM Sam McCall via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: sammccall
> Date: Mon Jan 22 13:05:00 2018
> New Revision: 323149
>
> URL: http://llvm.org/viewvc/llvm-project?rev=323149&view=rev
> Log:
> [clangd] Drop ~destructor completions - rarely helpful and work
> inconsistently
>
> Modified:
> clang-tools-extra/trunk/clangd/CodeComplete.cpp
> clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
>
> Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=323149&r1=323148&r2=323149&view=diff
>
> ==
> --- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
> +++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Mon Jan 22 13:05:00
> 2018
> @@ -361,6 +361,10 @@ struct CompletionRecorder : public CodeC
>(Result.Availability == CXAvailability_NotAvailable ||
> Result.Availability == CXAvailability_NotAccessible))
>  continue;
> +  // Destructor completion is rarely useful, and works inconsistently.
> +  // (s.^ completes ~string, but s.~st^ is an error).
> +  if (dyn_cast_or_null(Result.Declaration))
> +continue;
>Results.push_back(Result);
>  }
>}
>
> Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=323149&r1=323148&r2=323149&view=diff
>
> ==
> --- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
> (original)
> +++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Mon Jan
> 22 13:05:00 2018
> @@ -461,7 +461,7 @@ TEST(CompletionTest, NoDuplicates) {
>{cls("Adapter")});
>
>// Make sure there are no duplicate entries of 'Adapter'.
> -  EXPECT_THAT(Results.items, ElementsAre(Named("Adapter"),
> Named("~Adapter")));
> +  EXPECT_THAT(Results.items, ElementsAre(Named("Adapter")));
>  }
>
>  TEST(CompletionTest, ScopedNoIndex) {
>
>
> ___
> 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

[PATCH] D16403: Add scope information to CFG

[Maxim Ostapenko via Phabricator via cfe-commits]

m.ostapenko added a comment.

Hi Artem,

In https://reviews.llvm.org/D16403#989451, @NoQ wrote:

> Hmm. @m.ostapenko @szepet @mgehre - I think it might be a good time to figure 
> out if `ScopeBegin`/`ScopeEnd`, `LoopEntrance`/`LoopExit`, `LifetimeEnds`, 
> `AutomaticObjectDtor` elements work nicely together. How should they be 
> ordered with respect to each other?


Here are several observations about `ScopeBegin`/`ScopeEnd`, 
`LoopEntrance`/`LoopExit`, `LifetimeEnds`, `AutomaticObjectDtor` interaction:

1. `LifetimeEnds` and  `AutomaticObjectDtor` don't work together (I'm unable to 
tell why).
2. The difference between `ScopeBegin`/`ScopeEnd` and  `LifetimeEnds` was 
described by Devin several comments above, in short:
  - `LifetimeEnds` emits markers for when the lifetime of a C++ object in an 
automatic variable ends. For C++ objects with non-trivial destructors, this 
point is when the destructor is called. At this point the storage for the 
variable still exists, but what you can do with that storage is very restricted 
by the language because its contents have been destroyed.
  - `ScopeBegin`/`ScopeEnd` add markers for when the storage duration for the 
variable begins and ends. Hence I can conclude that `ScopeEnd` should reside 
after implicit destructors and `LifetimeEnds` markers in CFG.
3. `LoopEntrance`/`LoopExit` improve modelling of unrolled loops and I'm not 
sure whether scopes across iterations are the only thing that's modeled here.

> Is any of these scope representation a superset of another scope 
> representation, or maybe fully covered by other two or three other scope 
> representations?

Given my observations above, I'm not sure whether some representation can be 
fully covered by others -- all of them serve different purposes. Or perhaps I'm 
missing something?

> Would anybody be willing to produce some pictures (`-analyzer-checker 
> debug.ViewCFG` and attach here) with current and/or intended behavior?

I'm attaching two CFGs for the following example:

  void test_for_implicit_scope() {
for (A a; A b = a; )
  A c;
  }

  $ ./bin/clang -cc1 -fcxx-exceptions -fexceptions -analyze 
-analyzer-checker=debug.ViewCFG -analyzer-config cfg-scopes=true 
-analyzer-config cfg-loopexit=true -analyzer-config unroll-loops=true 
/tmp/scopes-cfg-output.cpp
  -> CFG-scopes-destructors-loopexit.dot
  
  $ ./bin/clang -cc1 -fcxx-exceptions -fexceptions -analyze 
-analyzer-checker=debug.ViewCFG -analyzer-config cfg-scopes=true 
-analyzer-config cfg-loopexit=true -analyzer-config cfg-lifetime=true 
-analyzer-config cfg-implicit-dtors=false /tmp/scopes-cfg-output.cpp
  -> CFG-scopes-loopexit-lifetime.dot{F5792940}
  
  {F5792939}



> Not sure, i guess `LifetimeEnds` is mostly used in `clang-tidy` so it does 
> not necessarily need to work together with analyzer-specific elements (or 
> maybe it's so great that we should switch to using it), but it would still be 
> great if we had a single scope representation which would be rich enough to 
> satisfy all needs.


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D16403: Add scope information to CFG

[Maxim Ostapenko via Phabricator via cfe-commits]

m.ostapenko added a comment.

Actually upload the files.
F5792949: CFG-scopes-destructors-loopexit.dot 


F5792948: CFG-scopes-loopexit-lifetime.dot 


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D42645: New simple Checker for mmap calls

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:1
+// MmapWriteExecChecker.cpp - Check for the prot argument
+//

Needs one of these at the top:

```
//===- MmapWriteExecChecker.cpp - Check the mmap prot argument 
---*- C++ -*-===//
```

Appropriately fitted to 80-cols.



Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:10
+//
+// This checker detects a common memory allocation security flaw.
+// Suppose 'unsigned int n' comes from an untrusted source. If the

This comment was lifted from `MallocOverflowSecurityChecker.cpp`, and doesn't 
accurately describe what *this* checker does.



Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:58
+
+if ((prot & (ProtWrite | ProtExec))) {
+  if (!BT) {

I assume you meant:


```
if ((prot & (ProtWrite | ProtExec)) == (ProtWrite | ProtExec)) {
```

?


Repository:
  rC Clang

https://reviews.llvm.org/D42645



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

r323664 - [analyzer] Fix -x language argument for C preprocessed sources

[Jonathan Roelofs via cfe-commits]

Author: jroelofs
Date: Mon Jan 29 08:37:53 2018
New Revision: 323664

URL: http://llvm.org/viewvc/llvm-project?rev=323664&view=rev
Log:
[analyzer] Fix -x language argument for C preprocessed sources

clang's -x option doesn't accept c-cpp-output as a language (even though
463eb6ab was merged, the driver still doesn't handle that).

This bug prevents testing C language projects when ccache is used.

Fixes #25851.

Investigation and patch by Dave Rigby.


Modified:
cfe/trunk/tools/scan-build/libexec/ccc-analyzer

Modified: cfe/trunk/tools/scan-build/libexec/ccc-analyzer
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/scan-build/libexec/ccc-analyzer?rev=323664&r1=323663&r2=323664&view=diff
==
--- cfe/trunk/tools/scan-build/libexec/ccc-analyzer (original)
+++ cfe/trunk/tools/scan-build/libexec/ccc-analyzer Mon Jan 29 08:37:53 2018
@@ -419,7 +419,7 @@ my %LangMap = (
   'cc'  => 'c++',
   'C'   => 'c++',
   'ii'  => 'c++-cpp-output',
-  'i'   => $IsCXX ? 'c++-cpp-output' : 'c-cpp-output',
+  'i'   => $IsCXX ? 'c++-cpp-output' : 'cpp-output',
   'm'   => 'objective-c',
   'mi'  => 'objective-c-cpp-output',
   'mm'  => 'objective-c++',
@@ -439,7 +439,7 @@ my %LangsAccepted = (
   "c" => 1,
   "c++" => 1,
   "objective-c++" => 1,
-  "c-cpp-output" => 1,
+  "cpp-output" => 1,
   "objective-c-cpp-output" => 1,
   "c++-cpp-output" => 1
 );


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

[PATCH] D42645: New simple Checker for mmap calls

[David CARLIER via Phabricator via cfe-commits]

devnexen updated this revision to Diff 131813.

Repository:
  rC Clang

https://reviews.llvm.org/D42645

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp

Index: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
@@ -0,0 +1,71 @@
+// MmapWriteExecChecker.cpp - Check for the prot argument -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This checker tests the 3rd argument of mmap's calls to check if
+// it is writable and executable in the same time. It's somehow
+// an optional checker since for example in JIT libraries it is pretty common.
+//
+//===--===//
+
+#include "ClangSACheckers.h"
+
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+
+using namespace clang;
+using namespace ento;
+using llvm::APSInt;
+
+namespace {
+class MmapWriteExecChecker : public Checker> {
+#ifdef __GLIBC__
+  static const int ProtWrite = 0x02;
+  static const int ProtExec  = 0x01;
+#else
+  static const int ProtWrite = 0x02;
+  static const int ProtExec  = 0x04;
+#endif
+  mutable std::unique_ptr BT;
+public:
+  void checkPostStmt(const CallExpr *S, CheckerContext &C) const;
+};
+}
+
+void MmapWriteExecChecker::checkPostStmt(const CallExpr *CE,
+ CheckerContext &C) const {
+  StringRef FName = C.getCalleeName(CE);
+  if (FName.empty())
+return;
+
+  if (FName == "mmap") {
+SVal ProtVal = C.getSVal(CE->getArg(2)); 
+Optional Prot = ProtVal.getAs();
+int64_t prot = Prot->getValue().getSExtValue();
+
+if ((prot & (ProtWrite | ProtExec)) == (ProtWrite | ProtExec)) {
+  if (!BT) {
+BT.reset(new BugType(this, "W^X check fail", "Write Exec prot flags set"));
+ExplodedNode *N = C.generateErrorNode();
+if (!N)
+  return;
+
+auto Report = llvm::make_unique(
+ *BT, "Both PROT_WRITE and PROT_EXEC flags had been set", N);
+Report->addRange(CE->getArg(2)->getSourceRange());
+C.emitReport(std::move(Report));
+  }
+}
+  }
+}
+
+void ento::registerMmapWriteExecChecker(CheckerManager &mgr) {
+  mgr.registerChecker();
+}
Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt
===
--- lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -50,6 +50,7 @@
   MallocOverflowSecurityChecker.cpp
   MallocSizeofChecker.cpp
   MisusedMovedObjectChecker.cpp
+  MmapWriteExecChecker.cpp
   MPI-Checker/MPIBugReporter.cpp
   MPI-Checker/MPIChecker.cpp
   MPI-Checker/MPIFunctionClassifier.cpp
Index: include/clang/StaticAnalyzer/Checkers/Checkers.td
===
--- include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -86,7 +86,7 @@
 
 // The APIModeling package is for checkers that model APIs and don't perform
 // any diagnostics. These checkers are always turned on; this package is
-// intended for API modeling that is not controlled by the the target triple.
+// intended for API modeling that is not controlled by the target triple.
 def APIModeling : Package<"apiModeling">, Hidden;
 def GoogleAPIModeling : Package<"google">, InPackage;
 
@@ -414,6 +414,10 @@
   HelpText<"Check for overflows in the arguments to malloc()">,
   DescFile<"MallocOverflowSecurityChecker.cpp">;
 
+def MmapWriteExecChecker : Checker<"MmapWriteExecChecker">,
+  HelpText<"Check if mmap() call is not both writable and executable">,
+  DescFile<"MmapWriteExecChecker.cpp">;
+
 } // end "alpha.security"
 
 //===--===//
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r323665 - [scan-build] Add an option to skip overriding CC and CXX make vars

[Jonathan Roelofs via cfe-commits]

Author: jroelofs
Date: Mon Jan 29 08:49:34 2018
New Revision: 323665

URL: http://llvm.org/viewvc/llvm-project?rev=323665&view=rev
Log:
[scan-build] Add an option to skip overriding CC and CXX make vars

Autoconf and some other systems tend to add essential compilation
options to CC (e.g. -std=gnu99). When running such an auto-generated
makefile, scan-build does not need to change CC and CXX as they are
already set to use ccc-analyzer by a configure script.

Implement a new option --keep-cc as was proposed in this discussion:
http://lists.llvm.org/pipermail/cfe-dev/2013-September/031832.html

Patch by Paul Fertser!

Modified:
cfe/trunk/tools/scan-build/bin/scan-build
cfe/trunk/www/analyzer/scan-build.html

Modified: cfe/trunk/tools/scan-build/bin/scan-build
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/scan-build/bin/scan-build?rev=323665&r1=323664&r2=323665&view=diff
==
--- cfe/trunk/tools/scan-build/bin/scan-build (original)
+++ cfe/trunk/tools/scan-build/bin/scan-build Mon Jan 29 08:49:34 2018
@@ -51,6 +51,7 @@ my %Options = (
   OutputDir => undef,# Parent directory to store HTML files.
   HtmlTitle => basename($CurrentDir)." - scan-build results",
   IgnoreErrors => 0, # Ignore build errors.
+  KeepCC => 0,   # Do not override CC and CXX make variables
   ViewResults => 0,  # View results when the build terminates.
   ExitStatusFoundBugs => 0,  # Exit status reflects whether bugs were found
   ShowDescription => 0,  # Display the description of the defect in the 
list
@@ -1062,6 +1063,7 @@ sub RunXcodebuild {
 sub RunBuildCommand {
   my $Args = shift;
   my $IgnoreErrors = shift;
+  my $KeepCC = shift;
   my $Cmd = $Args->[0];
   my $CCAnalyzer = shift;
   my $CXXAnalyzer = shift;
@@ -1099,8 +1101,10 @@ sub RunBuildCommand {
 unshift @$Args, $CXXAnalyzer;
   }
   elsif ($Cmd eq "make" or $Cmd eq "gmake" or $Cmd eq "mingw32-make") {
-AddIfNotPresent($Args, "CC=$CCAnalyzer");
-AddIfNotPresent($Args, "CXX=$CXXAnalyzer");
+if (!$KeepCC) {
+  AddIfNotPresent($Args, "CC=$CCAnalyzer");
+  AddIfNotPresent($Args, "CXX=$CXXAnalyzer");
+}
 if ($IgnoreErrors) {
   AddIfNotPresent($Args,"-k");
   AddIfNotPresent($Args,"-i");
@@ -1158,6 +1162,12 @@ OPTIONS:
currently supports make and xcodebuild. This is a convenience option; one
can specify this behavior directly using build options.
 
+ --keep-cc
+
+   Do not override CC and CXX make variables. Useful when running make in
+   autoconf-based (and similar) projects where configure can add extra flags
+   to those variables.
+
  --html-title [title]
  --html-title=[title]
 
@@ -1532,6 +1542,12 @@ sub ProcessArgs {
   next;
 }
 
+if ($arg eq "--keep-cc") {
+  shift @$Args;
+  $Options{KeepCC} = 1;
+  next;
+}
+
 if ($arg =~ /^--use-cc(=(.+))?$/) {
   shift @$Args;
   my $cc;
@@ -1838,8 +1854,8 @@ my %EnvVars = (
 );
 
 # Run the build.
-my $ExitStatus = RunBuildCommand(\@ARGV, $Options{IgnoreErrors}, $Cmd, $CmdCXX,
-\%EnvVars);
+my $ExitStatus = RunBuildCommand(\@ARGV, $Options{IgnoreErrors}, 
$Options{KeepCC},
+   $Cmd, $CmdCXX, \%EnvVars);
 
 if (defined $Options{OutputFormat}) {
   if ($Options{OutputFormat} =~ /plist/) {

Modified: cfe/trunk/www/analyzer/scan-build.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/www/analyzer/scan-build.html?rev=323665&r1=323664&r2=323665&view=diff
==
--- cfe/trunk/www/analyzer/scan-build.html (original)
+++ cfe/trunk/www/analyzer/scan-build.html Mon Jan 29 08:49:34 2018
@@ -248,7 +248,7 @@ you will probably need to run config
 
 
 $ scan-build ./configure
-$ scan-build make
+$ scan-build --keepk-cc make
 
 
 The reason configure also needs to be run through


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

[PATCH] D42645: New simple Checker for mmap calls

[David CARLIER via Phabricator via cfe-commits]

devnexen added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:10
+//
+// This checker detects a common memory allocation security flaw.
+// Suppose 'unsigned int n' comes from an untrusted source. If the

jroelofs wrote:
> This comment was lifted from `MallocOverflowSecurityChecker.cpp`, and doesn't 
> accurately describe what *this* checker does.
Exact sorry for that I created this patch in another machine and forgot to 
update. For sure I used MallocOverflowSecurityChecker header as template.



Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:58
+
+if ((prot & (ProtWrite | ProtExec))) {
+  if (!BT) {

jroelofs wrote:
> I assume you meant:
> 
> 
> ```
> if ((prot & (ProtWrite | ProtExec)) == (ProtWrite | ProtExec)) {
> ```
> 
> ?
True


Repository:
  rC Clang

https://reviews.llvm.org/D42645



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

[PATCH] D42641: [MinGW] Emit typeinfo locally for dllimported classes without key functions

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: lib/CodeGen/CGVTables.cpp:887-888
 
+  if (CGM.getTriple().isWindowsGNUEnvironment() && 
RD->hasAttr())
+return true;
+

Maybe a comment like "VTables of classes declared as dllimport are always 
considered to be external." ?



Comment at: lib/CodeGen/CGVTables.cpp:896-900
   // Otherwise, if the class is an instantiated template, the
   // vtable must be defined here.
   if (TSK == TSK_ImplicitInstantiation ||
   TSK == TSK_ExplicitInstantiationDefinition)
 return false;

It would be good to have tests for what would have happened if these paths got 
hit.


https://reviews.llvm.org/D42641



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

[PATCH] D42645: New simple Checker for mmap calls

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:1
+// MmapWriteExecChecker.cpp - Check for the prot argument
+//

jroelofs wrote:
> Needs one of these at the top:
> 
> ```
> //===- MmapWriteExecChecker.cpp - Check the mmap prot argument 
> ---*- C++ -*-===//
> ```
> 
> Appropriately fitted to 80-cols.
The format of the first line is important because some people's syntax 
highlighters rely on it.



Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:29
+class MmapWriteExecChecker : public Checker> {
+#ifdef __GLIBC__
+  static const int ProtWrite = 0x02;

This makes the checker's behavior depend on whether the host was built against 
glibc, however the target might not be the same as the host. You'll need to 
check the target's defines at checker-run-time, and not hardcode this.


Repository:
  rC Clang

https://reviews.llvm.org/D42645



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

[PATCH] D42493: [clang-format] Fix ObjC message arguments formatting.

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added inline comments.



Comment at: lib/Format/TokenAnnotator.cpp:419
   StartsObjCMethodExpr = true;
+  Left->ParameterCount = 0;
   Contexts.back().ColonIsObjCMethodExpr = true;

jolesiak wrote:
> benhamilton wrote:
> > What does this line do? Seems like it's initialized to 0 already, right?
> It is indeed initialized to 0.
> However, before 'Left' bracket is recognized as TT_ObjCMethodExpr it has a 
> different type assigned. Hence it gets updated here:
> https://github.com/llvm-mirror/clang/blob/release_60/lib/Format/TokenAnnotator.cpp#L495
> This assignment is due to fact that for other languages number of parameters 
> is calculated as (1 + number_of_commas).
I see. Could you add a comment explaining that?


Repository:
  rC Clang

https://reviews.llvm.org/D42493



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

[PATCH] D42639: [clang-move] Clever on handling header file which includes itself.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clang-move/ClangMove.cpp:708
+// Find old.h includes "old.h".
+if (AbsoluteOldHeader == AbsoluteOldHeader) {
+  OldHeaderIncludeRangeInHeader = IncludeFilenameRange;

This check is always true?



Comment at: clang-move/ClangMove.cpp:869
+if (Context->Spec.NewCC == NewFile && OldHeaderIncludeRangeInCC.isValid()) 
{
+  AllCode = AllCode.merge(clang::tooling::Replacements(
+  clang::tooling::Replacement(SM, OldHeaderIncludeRangeInCC,

I'd pull this into a lambda.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42639



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

[PATCH] D42343: [coroutines] Fix application of NRVO to Coroutine "Gro" or return object.

[Gor Nishanov via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D42343



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

[PATCH] D42645: New simple Checker for mmap calls

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin added a comment.

Hello David,

Do you have any results of this checker on the real code? If yes, could you 
please share them?
There are also some inline comments regarding implementation.




Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:42
+
+void MmapWriteExecChecker::checkPostStmt(const CallExpr *CE,
+ CheckerContext &C) const {

For analysis of function arguments, PreCall callback is more suitable because 
it is called earlier.



Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:49
+  if (FName == "mmap") {
+SVal ProtVal = C.getSVal(CE->getArg(2)); 
+Optional Prot = ProtVal.getAs();

We need to check that the function call has at least three arguments to avoid 
crashing on weird redeclarations like `void mmap()`.



Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:51
+Optional Prot = ProtVal.getAs();
+int64_t prot = Prot->getValue().getSExtValue();
+

Please follow LLVM naming conventions: variable names should start with capital.



Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:54
+if ((prot & (ProtWrite | ProtExec)) == (ProtWrite | ProtExec)) {
+  if (!BT) {
+BT.reset(new BugType(this, "W^X check fail", "Write Exec prot flags 
set"));

Looks like this check is written in the way that allows to emit warning only 
once. Did you mean:
```
if (!BT)
   BT.reset(new BugType(this, "W^X check fail", "Write Exec prot flags );
ExplodedNode *N = C.generateErrorNode();
if (!N)
  return;
...
```
?



Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:55
+  if (!BT) {
+BT.reset(new BugType(this, "W^X check fail", "Write Exec prot flags 
set"));
+ExplodedNode *N = C.generateErrorNode();

Could you please give more informative warning message describing the situation 
and why is it harmful? Note that if user has more information, he can fix the 
problem faster.


Repository:
  rC Clang

https://reviews.llvm.org/D42645



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

r323668 - [clang-format] Fix bug where -dump-config failed on ObjC header

[Ben Hamilton via cfe-commits]

Author: benhamilton
Date: Mon Jan 29 09:36:43 2018
New Revision: 323668

URL: http://llvm.org/viewvc/llvm-project?rev=323668&view=rev
Log:
[clang-format] Fix bug where -dump-config failed on ObjC header

Summary:
`clang-format -dump-config path/to/file.h` never passed
anything for the Code parameter to clang::format::getStyle().

This meant the logic to guess Objective-C from the contents
of a .h file never worked, because LibFormat didn't have the
code to work with.

With this fix, we now correctly read in the contents of the
file if possible with -dump-config.

I had to update the lit config for test/Format/ because
the default config ignores .h files.

Test Plan: make -j12 check-clang

Reviewers: jolesiak, krasimir

Reviewed By: jolesiak, krasimir

Subscribers: Wizard, klimek, cfe-commits, djasper

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

Added:
cfe/trunk/test/Format/dump-config-cxx.h
cfe/trunk/test/Format/dump-config-objc.h
cfe/trunk/test/Format/lit.local.cfg
Modified:
cfe/trunk/tools/clang-format/ClangFormat.cpp

Added: cfe/trunk/test/Format/dump-config-cxx.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Format/dump-config-cxx.h?rev=323668&view=auto
==
--- cfe/trunk/test/Format/dump-config-cxx.h (added)
+++ cfe/trunk/test/Format/dump-config-cxx.h Mon Jan 29 09:36:43 2018
@@ -0,0 +1,3 @@
+// RUN: clang-format -dump-config %s | FileCheck %s
+
+// CHECK: Language: Cpp

Added: cfe/trunk/test/Format/dump-config-objc.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Format/dump-config-objc.h?rev=323668&view=auto
==
--- cfe/trunk/test/Format/dump-config-objc.h (added)
+++ cfe/trunk/test/Format/dump-config-objc.h Mon Jan 29 09:36:43 2018
@@ -0,0 +1,5 @@
+// RUN: clang-format -dump-config %s | FileCheck %s
+
+// CHECK: Language: ObjC
+@interface Foo
+@end

Added: cfe/trunk/test/Format/lit.local.cfg
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Format/lit.local.cfg?rev=323668&view=auto
==
--- cfe/trunk/test/Format/lit.local.cfg (added)
+++ cfe/trunk/test/Format/lit.local.cfg Mon Jan 29 09:36:43 2018
@@ -0,0 +1,4 @@
+# Suffixes supported by clang-format.
+config.suffixes = ['.c', '.cc', '.cpp', '.h', '.m', '.mm', '.java', '.js',
+   '.ts', '.proto', '.protodevel', '.pb.txt', '.textproto',
+   '.textpb', '.asciipb', '.td']

Modified: cfe/trunk/tools/clang-format/ClangFormat.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-format/ClangFormat.cpp?rev=323668&r1=323667&r2=323668&view=diff
==
--- cfe/trunk/tools/clang-format/ClangFormat.cpp (original)
+++ cfe/trunk/tools/clang-format/ClangFormat.cpp Mon Jan 29 09:36:43 2018
@@ -357,10 +357,27 @@ int main(int argc, const char **argv) {
   }
 
   if (DumpConfig) {
+StringRef FileName;
+std::unique_ptr Code;
+if (FileNames.empty()) {
+  // We can't read the code to detect the language if there's no
+  // file name, so leave Code empty here.
+  FileName = AssumeFileName;
+} else {
+  // Read in the code in case the filename alone isn't enough to
+  // detect the language.
+  ErrorOr> CodeOrErr =
+  MemoryBuffer::getFileOrSTDIN(FileNames[0]);
+  if (std::error_code EC = CodeOrErr.getError()) {
+llvm::errs() << EC.message() << "\n";
+return 1;
+  }
+  FileName = (FileNames[0] == "-") ? AssumeFileName : FileNames[0];
+  Code = std::move(CodeOrErr.get());
+}
 llvm::Expected FormatStyle =
-clang::format::getStyle(
-Style, FileNames.empty() ? AssumeFileName : FileNames[0],
-FallbackStyle);
+clang::format::getStyle(Style, FileName, FallbackStyle,
+Code ? Code->getBuffer() : "");
 if (!FormatStyle) {
   llvm::errs() << llvm::toString(FormatStyle.takeError()) << "\n";
   return 1;


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

[PATCH] D42395: [clang-format] Fix bug where -dump-config failed on ObjC header

[Ben Hamilton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
benhamilton marked an inline comment as done.
Closed by commit rC323668: [clang-format] Fix bug where -dump-config failed on 
ObjC header (authored by benhamilton, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D42395?vs=131470&id=131819#toc

Repository:
  rC Clang

https://reviews.llvm.org/D42395

Files:
  test/Format/dump-config-cxx.h
  test/Format/dump-config-objc.h
  test/Format/lit.local.cfg
  tools/clang-format/ClangFormat.cpp


Index: tools/clang-format/ClangFormat.cpp
===
--- tools/clang-format/ClangFormat.cpp
+++ tools/clang-format/ClangFormat.cpp
@@ -357,10 +357,27 @@
   }
 
   if (DumpConfig) {
+StringRef FileName;
+std::unique_ptr Code;
+if (FileNames.empty()) {
+  // We can't read the code to detect the language if there's no
+  // file name, so leave Code empty here.
+  FileName = AssumeFileName;
+} else {
+  // Read in the code in case the filename alone isn't enough to
+  // detect the language.
+  ErrorOr> CodeOrErr =
+  MemoryBuffer::getFileOrSTDIN(FileNames[0]);
+  if (std::error_code EC = CodeOrErr.getError()) {
+llvm::errs() << EC.message() << "\n";
+return 1;
+  }
+  FileName = (FileNames[0] == "-") ? AssumeFileName : FileNames[0];
+  Code = std::move(CodeOrErr.get());
+}
 llvm::Expected FormatStyle =
-clang::format::getStyle(
-Style, FileNames.empty() ? AssumeFileName : FileNames[0],
-FallbackStyle);
+clang::format::getStyle(Style, FileName, FallbackStyle,
+Code ? Code->getBuffer() : "");
 if (!FormatStyle) {
   llvm::errs() << llvm::toString(FormatStyle.takeError()) << "\n";
   return 1;
Index: test/Format/lit.local.cfg
===
--- test/Format/lit.local.cfg
+++ test/Format/lit.local.cfg
@@ -0,0 +1,4 @@
+# Suffixes supported by clang-format.
+config.suffixes = ['.c', '.cc', '.cpp', '.h', '.m', '.mm', '.java', '.js',
+   '.ts', '.proto', '.protodevel', '.pb.txt', '.textproto',
+   '.textpb', '.asciipb', '.td']
Index: test/Format/dump-config-objc.h
===
--- test/Format/dump-config-objc.h
+++ test/Format/dump-config-objc.h
@@ -0,0 +1,5 @@
+// RUN: clang-format -dump-config %s | FileCheck %s
+
+// CHECK: Language: ObjC
+@interface Foo
+@end
Index: test/Format/dump-config-cxx.h
===
--- test/Format/dump-config-cxx.h
+++ test/Format/dump-config-cxx.h
@@ -0,0 +1,3 @@
+// RUN: clang-format -dump-config %s | FileCheck %s
+
+// CHECK: Language: Cpp


Index: tools/clang-format/ClangFormat.cpp
===
--- tools/clang-format/ClangFormat.cpp
+++ tools/clang-format/ClangFormat.cpp
@@ -357,10 +357,27 @@
   }
 
   if (DumpConfig) {
+StringRef FileName;
+std::unique_ptr Code;
+if (FileNames.empty()) {
+  // We can't read the code to detect the language if there's no
+  // file name, so leave Code empty here.
+  FileName = AssumeFileName;
+} else {
+  // Read in the code in case the filename alone isn't enough to
+  // detect the language.
+  ErrorOr> CodeOrErr =
+  MemoryBuffer::getFileOrSTDIN(FileNames[0]);
+  if (std::error_code EC = CodeOrErr.getError()) {
+llvm::errs() << EC.message() << "\n";
+return 1;
+  }
+  FileName = (FileNames[0] == "-") ? AssumeFileName : FileNames[0];
+  Code = std::move(CodeOrErr.get());
+}
 llvm::Expected FormatStyle =
-clang::format::getStyle(
-Style, FileNames.empty() ? AssumeFileName : FileNames[0],
-FallbackStyle);
+clang::format::getStyle(Style, FileName, FallbackStyle,
+Code ? Code->getBuffer() : "");
 if (!FormatStyle) {
   llvm::errs() << llvm::toString(FormatStyle.takeError()) << "\n";
   return 1;
Index: test/Format/lit.local.cfg
===
--- test/Format/lit.local.cfg
+++ test/Format/lit.local.cfg
@@ -0,0 +1,4 @@
+# Suffixes supported by clang-format.
+config.suffixes = ['.c', '.cc', '.cpp', '.h', '.m', '.mm', '.java', '.js',
+   '.ts', '.proto', '.protodevel', '.pb.txt', '.textproto',
+   '.textpb', '.asciipb', '.td']
Index: test/Format/dump-config-objc.h
===
--- test/Format/dump-config-objc.h
+++ test/Format/dump-config-objc.h
@@ -0,0 +1,5 @@
+// RUN: clang-format -dump-config %s | FileCheck %s
+
+// CHECK: Language: ObjC
+@interface Foo
+@end
Index: test/Format/dump-config-cxx.h

[PATCH] D42464: add prefix with '_' support for property name. Corresponding apple dev doc: https://developer.apple.com/library/content/qa/qa1908/_index.html

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton requested changes to this revision.
benhamilton added inline comments.
This revision now requires changes to proceed.



Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:115
+
+bool prefixedPropertyNameMatches(const llvm::StringRef &PropertyName,
+ const std::vector &Acronyms) {

Wizard wrote:
> hokein wrote:
> > Wizard wrote:
> > > hokein wrote:
> > > > no need to pass const reference of `llvm::StringRef`. `StringRef` is 
> > > > cheap.
> > > The original property name I directly get from ast is a const. If I 
> > > remove the const here, I will have to make a copy of the property name 
> > > before calling this.
> > > I prefer keep this const to save that copy :-)
> > Why? `MatchedDecl->getName()` returns `llvm::StringRef`. As the name 
> > indicates, StringRef is a const reference of String, using StringRef is 
> > sufficient.
> > 
> > The same to ArrayRef. So the function is like `bool 
> > prefixedPropertyNameMatches(llvm::StringRef PropertyName, 
> > llvm::ArrayRef Acronyms)`.
> > 
> > 
> If I remove the const in the method, compiler will have error of "candidate 
> function not viable: expects an l-value for 1st argument". I believe it 
> cannot accept a const reference as arg if the definition is var.
I think hokein@ is saying you should change:

  const llvm::StringRef &

to:

  llvm::StringRef

Note removing both `const` and `&`, not just removing `const`. This is because 
there is no need to pass these by reference, they are already a reference.

Same with `const llvm::ArrayRef &`, it should just be `llvm::ArrayRef`.



Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:151
+  hasCategoryPropertyPrefix(MatchedDecl->getName())) {
+const auto *CategoryDecl = (const ObjCCategoryDecl *)(DeclContext);
+if (!prefixedPropertyNameMatches(MatchedDecl->getName(), SpecialAcronyms) 
||

Wizard wrote:
> hokein wrote:
> > Wizard wrote:
> > > hokein wrote:
> > > > Consider using `const auto* CategoryDecl = 
> > > > dyn_cast(DeclContext)`, we can get rid of this cast, 
> > > > and `NodeKind` variable.
> > > Tried that before but I encountered 2 issues:
> > > 1. 'clang::DeclContext' is not polymorphic
> > > 2. cannot use dynamic_cast with -fno-rtti
> > > which are preventing me from using dynamic_cast
> > Sorry, I mean `llvm::dyn_cast` here, it should work.
> It is not working either. It says "error: static_cast from 'clang::Decl *' to 
> 'const clang::ObjCCategoryDecl *const *' is not allowed" though I have no 
> idea why it is regarded as a static cast.
> I am using it like this:
> const auto *CategoryDecl = llvm::dyn_cast *>(DeclContext);
You definitely don't want to use a C-style cast. My guess is adding const is 
not part of what dyn_cast<> does, so you can probably remove that from the 
dyn_cast<>. (You can of course assign to a const pointer if you want.)




Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:125
+  llvm::Regex(llvm::StringRef(validPropertyNameRegex(Acronyms, false)));
+  return RegexExp.match(llvm::StringRef(PropertyName.substr(Start + 1)));
 }

No need to construct another llvm::StringRef(), StringRef::substr() returns a 
StringRef already.



Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42464



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

[PATCH] D42614: AST: support ObjC lifetime qualifiers in MS ABI

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a comment.

Hmm, the only thing that we could fake would be namespaces on the parameter 
types.  Is that better?  I'm not tied to re-using the existing mangling.


Repository:
  rC Clang

https://reviews.llvm.org/D42614



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

[PATCH] D42649: [clang-format] Add more tests for ObjC protocol list formatting behavior

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton created this revision.
benhamilton added reviewers: krasimir, jolesiak.
Herald added subscribers: cfe-commits, klimek.

The existing unit tests in FormatTestObjC.cpp didn't fully cover
all the cases for protocol confirmance list formatting.

This extends the unit tests to more cases of protocol
conformance list formatting, especially how the behavior changes
when `BinPackParameters` changes from `true` (the default) to `false`.

Test Plan: make -j12 FormatTests && \

  ./tools/clang/unittests/Format/FormatTests --gtest_filter=FormatTestObjC.\*


Repository:
  rC Clang

https://reviews.llvm.org/D42649

Files:
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -276,6 +276,20 @@
"+ (id)init;\n"
"@end");
 
+  Style.ColumnLimit = 40;
+  verifyFormat("@interface c () <\n"
+   "c, c,\n"
+   "c, c> {\n"
+   "}");
+
+  Style.BinPackParameters = false;
+  verifyFormat("@interface d () <\n"
+   "d,\n"
+   "d,\n"
+   "d,\n"
+   "d> {\n"
+   "}");
+
   Style = getGoogleStyle(FormatStyle::LK_ObjC);
   verifyFormat("@interface Foo : NSObject  {\n"
" @public\n"


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -276,6 +276,20 @@
"+ (id)init;\n"
"@end");
 
+  Style.ColumnLimit = 40;
+  verifyFormat("@interface c () <\n"
+   "c, c,\n"
+   "c, c> {\n"
+   "}");
+
+  Style.BinPackParameters = false;
+  verifyFormat("@interface d () <\n"
+   "d,\n"
+   "d,\n"
+   "d,\n"
+   "d> {\n"
+   "}");
+
   Style = getGoogleStyle(FormatStyle::LK_ObjC);
   verifyFormat("@interface Foo : NSObject  {\n"
" @public\n"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42578: [AMDGPU] Add ds_fadd, ds_fmin, ds_fmax builtins functions

[Daniil Fukalov via Phabricator via cfe-commits]

dfukalov updated this revision to Diff 131820.
dfukalov added a comment.

fixed builtins descriptions


Repository:
  rC Clang

https://reviews.llvm.org/D42578

Files:
  include/clang/Basic/BuiltinsAMDGPU.def
  test/CodeGenOpenCL/builtins-amdgcn-vi.cl


Index: test/CodeGenOpenCL/builtins-amdgcn-vi.cl
===
--- test/CodeGenOpenCL/builtins-amdgcn-vi.cl
+++ test/CodeGenOpenCL/builtins-amdgcn-vi.cl
@@ -89,3 +89,23 @@
   *out = __builtin_amdgcn_mov_dpp(src, 0, 0, 0, false);
 }
 
+// CHECK-LABEL: @test_ds_fadd
+// CHECK: call float @llvm.amdgcn.ds.fadd(float addrspace(3)* %out, float 
%src, i32 0, i32 0, i1 false)
+void test_ds_fadd(local float *out, float src)
+{
+  *out = __builtin_amdgcn_ds_fadd(out, src, 0, 0, false);
+}
+
+// CHECK-LABEL: @test_ds_fmin
+// CHECK: call float @llvm.amdgcn.ds.fmin(float addrspace(3)* %out, float 
%src, i32 0, i32 0, i1 false)
+void test_ds_fmin(local float *out, float src)
+{
+  *out = __builtin_amdgcn_ds_fmin(out, src, 0, 0, false);
+}
+
+// CHECK-LABEL: @test_ds_fmax
+// CHECK: call float @llvm.amdgcn.ds.fmax(float addrspace(3)* %out, float 
%src, i32 0, i32 0, i1 false)
+void test_ds_fmax(local float *out, float src)
+{
+  *out = __builtin_amdgcn_ds_fmax(out, src, 0, 0, false);
+}
Index: include/clang/Basic/BuiltinsAMDGPU.def
===
--- include/clang/Basic/BuiltinsAMDGPU.def
+++ include/clang/Basic/BuiltinsAMDGPU.def
@@ -93,6 +93,9 @@
 BUILTIN(__builtin_amdgcn_readfirstlane, "ii", "nc")
 BUILTIN(__builtin_amdgcn_readlane, "iii", "nc")
 BUILTIN(__builtin_amdgcn_fmed3f, "", "nc")
+BUILTIN(__builtin_amdgcn_ds_fadd, "ff*3fiib", "n")
+BUILTIN(__builtin_amdgcn_ds_fmin, "ff*3fiib", "n")
+BUILTIN(__builtin_amdgcn_ds_fmax, "ff*3fiib", "n")
 
 
//===--===//
 // VI+ only builtins.


Index: test/CodeGenOpenCL/builtins-amdgcn-vi.cl
===
--- test/CodeGenOpenCL/builtins-amdgcn-vi.cl
+++ test/CodeGenOpenCL/builtins-amdgcn-vi.cl
@@ -89,3 +89,23 @@
   *out = __builtin_amdgcn_mov_dpp(src, 0, 0, 0, false);
 }
 
+// CHECK-LABEL: @test_ds_fadd
+// CHECK: call float @llvm.amdgcn.ds.fadd(float addrspace(3)* %out, float %src, i32 0, i32 0, i1 false)
+void test_ds_fadd(local float *out, float src)
+{
+  *out = __builtin_amdgcn_ds_fadd(out, src, 0, 0, false);
+}
+
+// CHECK-LABEL: @test_ds_fmin
+// CHECK: call float @llvm.amdgcn.ds.fmin(float addrspace(3)* %out, float %src, i32 0, i32 0, i1 false)
+void test_ds_fmin(local float *out, float src)
+{
+  *out = __builtin_amdgcn_ds_fmin(out, src, 0, 0, false);
+}
+
+// CHECK-LABEL: @test_ds_fmax
+// CHECK: call float @llvm.amdgcn.ds.fmax(float addrspace(3)* %out, float %src, i32 0, i32 0, i1 false)
+void test_ds_fmax(local float *out, float src)
+{
+  *out = __builtin_amdgcn_ds_fmax(out, src, 0, 0, false);
+}
Index: include/clang/Basic/BuiltinsAMDGPU.def
===
--- include/clang/Basic/BuiltinsAMDGPU.def
+++ include/clang/Basic/BuiltinsAMDGPU.def
@@ -93,6 +93,9 @@
 BUILTIN(__builtin_amdgcn_readfirstlane, "ii", "nc")
 BUILTIN(__builtin_amdgcn_readlane, "iii", "nc")
 BUILTIN(__builtin_amdgcn_fmed3f, "", "nc")
+BUILTIN(__builtin_amdgcn_ds_fadd, "ff*3fiib", "n")
+BUILTIN(__builtin_amdgcn_ds_fmin, "ff*3fiib", "n")
+BUILTIN(__builtin_amdgcn_ds_fmax, "ff*3fiib", "n")
 
 //===--===//
 // VI+ only builtins.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [clang-tools-extra] r323149 - [clangd] Drop ~destructor completions - rarely helpful and work inconsistently

[Sam McCall via cfe-commits]

On Mon, Jan 29, 2018 at 5:31 PM, David Blaikie  wrote:

> Any chance of making this a really low priority completion?
>
(Just for clarity - this is a postfilter in clangd only, we haven't changed
the behavior of the Sema or libclang APIs)

We certainly have considered downranking such items, maybe it's the right
thing, but it's not without issues on the UX side:
 - It's far from the only candidate, other things that people want "at the
bottom" include inaccessible and unavailable members, injected type names,
operators... it gets crowded, so you still have to decide how to rank them.
 - LSP doesn't (currently) have any affordance to "grey out" results and
show users "this is the last good result, the ones below here are unlikely
to be useful". So the important end-of-list signal is lost.
There's also a bit of implementation complexity in having
second/third/fourth "tiers" of results - it adds invariants to the scoring
logic that make it harder to understand and modify.

(maybe just leaving in a FIXME or expected-fail check of some kind if it's
> not practical to implement it right now) For those handful of times when
> you actually want to do this.
>
re: practical to implement, there are a few problems with it, apart from
being rarely useful:
 - It completes after "foo.", but not "foo.~F", I guess because the parser
is in the wrong state
 - it completes ~basic_string, but not ~string

We do indirectly test that destructor completions are turned off in clangd.
I don't really know where to add the FIXMEs. I can add XFAIL tests to
c-index-test I think, is anyone likely to go looking for them?

On Mon, Jan 22, 2018 at 1:06 PM Sam McCall via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: sammccall
>> Date: Mon Jan 22 13:05:00 2018
>> New Revision: 323149
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=323149&view=rev
>> Log:
>> [clangd] Drop ~destructor completions - rarely helpful and work
>> inconsistently
>>
>> Modified:
>> clang-tools-extra/trunk/clangd/CodeComplete.cpp
>> clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
>>
>> Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/
>> trunk/clangd/CodeComplete.cpp?rev=323149&r1=323148&r2=323149&view=diff
>> 
>> ==
>> --- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
>> +++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Mon Jan 22 13:05:00
>> 2018
>> @@ -361,6 +361,10 @@ struct CompletionRecorder : public CodeC
>>(Result.Availability == CXAvailability_NotAvailable ||
>> Result.Availability == CXAvailability_NotAccessible))
>>  continue;
>> +  // Destructor completion is rarely useful, and works
>> inconsistently.
>> +  // (s.^ completes ~string, but s.~st^ is an error).
>> +  if (dyn_cast_or_null(Result.Declaration))
>> +continue;
>>Results.push_back(Result);
>>  }
>>}
>>
>> Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/
>> trunk/unittests/clangd/CodeCompleteTests.cpp?rev=
>> 323149&r1=323148&r2=323149&view=diff
>> 
>> ==
>> --- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
>> (original)
>> +++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Mon
>> Jan 22 13:05:00 2018
>> @@ -461,7 +461,7 @@ TEST(CompletionTest, NoDuplicates) {
>>{cls("Adapter")});
>>
>>// Make sure there are no duplicate entries of 'Adapter'.
>> -  EXPECT_THAT(Results.items, ElementsAre(Named("Adapter"),
>> Named("~Adapter")));
>> +  EXPECT_THAT(Results.items, ElementsAre(Named("Adapter")));
>>  }
>>
>>  TEST(CompletionTest, ScopedNoIndex) {
>>
>>
>> ___
>> 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

[PATCH] D42650: [clang-format] New format param BinPackObjCProtocolList

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton created this revision.
benhamilton added reviewers: krasimir, jolesiak, stephanemoore.

This is an alternative approach to https://reviews.llvm.org/D42014 after some
investigation by stephanemoore@ and myself.

Previously, the format parameter `BinPackParameters` controlled both
C function parameter list bin-packing and Objective-C protocol conformance
list bin-packing.

We found in the Google style, some teams were changing
`BinPackParameters` from its default (`true`) to `false` so they could
lay out Objective-C protocol conformance list items one-per-line
instead of bin-packing them into as few lines as possible.

To allow teams to use one-per-line Objective-C protocol lists without
changing bin-packing for other areas like C function parameter lists,
this diff introduces a new LibFormat parameter
`BinPackObjCProtocolList` to control the behavior just for ObjC
protocol conformance lists.

The new parameter is an enum which defaults to `Auto` to keep the
previous behavior (delegating to `BinPackParameters`).

For the `google` style, we set it to `Never` so we use one-per-line
protocol conformance lists instead of bin-packing.

Depends On https://reviews.llvm.org/D42649

Test Plan: New tests added. make -j12 FormatTests && 
./tools/clang/unittests/Format/FormatTests


Repository:
  rC Clang

https://reviews.llvm.org/D42650

Files:
  include/clang/Format/Format.h
  lib/Format/ContinuationIndenter.cpp
  lib/Format/Format.cpp
  unittests/Format/FormatTestObjC.cpp

Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -281,15 +281,28 @@
"c, c,\n"
"c, c> {\n"
"}");
-
-  Style.BinPackParameters = false;
+  Style.BinPackObjCProtocolList = FormatStyle::BPS_Never;
   verifyFormat("@interface d () <\n"
"d,\n"
"d,\n"
"d,\n"
"d> {\n"
"}");
 
+  Style.BinPackParameters = false;
+  Style.BinPackObjCProtocolList = FormatStyle::BPS_Auto;
+  verifyFormat("@interface e () <\n"
+   "e,\n"
+   "e,\n"
+   "e,\n"
+   "e> {\n"
+   "}");
+  Style.BinPackObjCProtocolList = FormatStyle::BPS_Always;
+  verifyFormat("@interface f () <\n"
+   "f, f,\n"
+   "f, f> {\n"
+   "}");
+
   Style = getGoogleStyle(FormatStyle::LK_ObjC);
   verifyFormat("@interface Foo : NSObject  {\n"
" @public\n"
@@ -309,13 +322,16 @@
   verifyFormat("@interface Foo (HackStuff) \n"
"+ (id)init;\n"
"@end");
-  Style.BinPackParameters = false;
-  Style.ColumnLimit = 80;
-  verifyFormat("@interface a () <\n"
-   "a,\n"
-   ",\n"
-   "aa,\n"
-   "> {\n"
+  Style.ColumnLimit = 40;
+  // BinPackParameters should be true by default.
+  verifyFormat("void (int e, int e,\n"
+   "  int e, int e);\n");
+  // BinPackObjCProtocolList should be BPS_Never by default.
+  verifyFormat("@interface f () <\n"
+   "f,\n"
+   "f,\n"
+   "f,\n"
+   "f> {\n"
"}");
 }
 
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -105,6 +105,14 @@
   }
 };
 
+template <> struct ScalarEnumerationTraits {
+  static void enumeration(IO &IO, FormatStyle::BinPackStyle &Value) {
+IO.enumCase(Value, "Auto", FormatStyle::BPS_Auto);
+IO.enumCase(Value, "Always", FormatStyle::BPS_Always);
+IO.enumCase(Value, "Never", FormatStyle::BPS_Never);
+  }
+};
+
 template <> struct ScalarEnumerationTraits {
   static void enumeration(IO &IO, FormatStyle::BinaryOperatorStyle &Value) {
 IO.enumCase(Value, "All", FormatStyle::BOS_All);
@@ -323,6 +331,7 @@
Style.AlwaysBreakTemplateDeclarations);
 IO.mapOptional("BinPackArguments", Style.BinPackArguments);
 IO.mapOptional("BinPackParameters", Style.BinPackParameters);
+IO.mapOptional("BinPackObjCProtocolList", Style.BinPackObjCProtocolList);
 IO.mapOptional("BraceWrapping", Style.BraceWrapping);
 IO.mapOptional("BreakBeforeBinaryOperators",
Style.BreakBeforeBinaryOperators);
@@ -599,6 +608,7 @@

[PATCH] D42014: Disable BinPackArguments and BinPackParameters in Google Objective-C style ⚙️

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added a comment.

Check out https://reviews.llvm.org/D42650 for an alternative approach.


https://reviews.llvm.org/D42014



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

[PATCH] D42493: [clang-format] Fix ObjC message arguments formatting.

[Jacek Olesiak via Phabricator via cfe-commits]

jolesiak updated this revision to Diff 131825.
jolesiak added a comment.

- Add comment explaining ParameterCount reset.


Repository:
  rC Clang

https://reviews.llvm.org/D42493

Files:
  lib/Format/ContinuationIndenter.cpp
  lib/Format/FormatToken.h
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTestObjC.cpp

Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -655,6 +655,31 @@
"ofSize:aa:bbb\n"
"  atOrigin:cc:dd];");
 
+  // Inline block as a first argument.
+  verifyFormat("[object justBlock:^{\n"
+   "  a = 42;\n"
+   "}];");
+  verifyFormat("[object\n"
+   "justBlock:^{\n"
+   "  a = 42;\n"
+   "}\n"
+   " notBlock:42\n"
+   "a:42];");
+  verifyFormat("[object\n"
+   "firstBlock:^{\n"
+   "  a = 42;\n"
+   "}\n"
+   "blockWithLongerName:^{\n"
+   "  a = 42;\n"
+   "}];");
+  verifyFormat("[object\n"
+   "blockWithLongerName:^{\n"
+   "  a = 42;\n"
+   "}\n"
+   "secondBlock:^{\n"
+   "  a = 42;\n"
+   "}];");
+
   Style.ColumnLimit = 70;
   verifyFormat(
   "void f() {\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -401,6 +401,8 @@
 if (Contexts.back().FirstObjCSelectorName) {
   Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName =
   Contexts.back().LongestObjCSelectorName;
+  Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts =
+  Left->ParameterCount;
   if (Left->BlockParameterCount > 1)
 Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName = 0;
 }
@@ -414,6 +416,11 @@
   TT_DesignatedInitializerLSquare)) {
   Left->Type = TT_ObjCMethodExpr;
   StartsObjCMethodExpr = true;
+  // ParameterCount might have been set to 1 before expression was
+  // recognized as ObjCMethodExpr (as '1 + number of commas' formula is
+  // used for other expression types). Parameter counter has to be,
+  // therefore, reset to 0.
+  Left->ParameterCount = 0;
   Contexts.back().ColonIsObjCMethodExpr = true;
   if (Parent && Parent->is(tok::r_paren))
 Parent->Type = TT_CastRParen;
@@ -486,7 +493,10 @@
   void updateParameterCount(FormatToken *Left, FormatToken *Current) {
 if (Current->is(tok::l_brace) && Current->BlockKind == BK_Block)
   ++Left->BlockParameterCount;
-if (Current->is(tok::comma)) {
+if (Left->Type == TT_ObjCMethodExpr) {
+  if (Current->is(tok::colon))
+++Left->ParameterCount;
+} else if (Current->is(tok::comma)) {
   ++Left->ParameterCount;
   if (!Left->Role)
 Left->Role.reset(new CommaSeparatedList(Style));
Index: lib/Format/FormatToken.h
===
--- lib/Format/FormatToken.h
+++ lib/Format/FormatToken.h
@@ -240,6 +240,10 @@
   /// e.g. because several of them are block-type.
   unsigned LongestObjCSelectorName = 0;
 
+  /// \brief How many parts ObjC selector have (i.e. how many parameters method
+  /// has).
+  unsigned ObjCSelectorNameParts = 0;
+
   /// \brief Stores the number of required fake parentheses and the
   /// corresponding operator precedence.
   ///
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -266,6 +266,11 @@
 return true;
   if (Previous.is(tok::semi) && State.LineContainsContinuedForLoopSection)
 return true;
+  if (Style.Language == FormatStyle::LK_ObjC &&
+  Current.ObjCSelectorNameParts > 1 &&
+  Current.startsSequence(TT_SelectorName, tok::colon, tok::caret)) {
+return true;
+  }
   if ((startsNextParameter(Current, Style) || Previous.is(tok::semi) ||
(Previous.is(TT_TemplateCloser) && Current.is(TT_StartOfName) &&
 Style.isCpp() &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42614: AST: support ObjC lifetime qualifiers in MS ABI

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D42614#990808, @compnerd wrote:

> Hmm, the only thing that we could fake would be namespaces on the parameter 
> types.  Is that better?  I'm not tied to re-using the existing mangling.


I'm just worried about re-using the existing mangling causing a problem in the 
long run.  If you're certain that it would never be sensical to talk about e.g. 
a cli::pin_ptr<__strong id>, then ok.


Repository:
  rC Clang

https://reviews.llvm.org/D42614



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

[PATCH] D42614: AST: support ObjC lifetime qualifiers in MS ABI

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a comment.

Yeah, Im afraid I cannot definitely say that will never be sensible.  I think I 
would, in fact, prefer an alternative.  I'm just not sure what is a good viable 
alternative, especially since we are constrained in what the grammar accepts.  
The desugaring of the `id` and `Class` types is the saving grace here that 
grants us some ability to tweak the mangling.


Repository:
  rC Clang

https://reviews.llvm.org/D42614



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

[PATCH] D42493: [clang-format] Fix ObjC message arguments formatting.

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton accepted this revision.
benhamilton added a comment.

shipit 



Repository:
  rC Clang

https://reviews.llvm.org/D42493



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

[libclc] r323677 - math.h: Set HAVE_HW_FMA32 based on compiler provided macro

[Jan Vesely via cfe-commits]

Author: jvesely
Date: Mon Jan 29 11:05:08 2018
New Revision: 323677

URL: http://llvm.org/viewvc/llvm-project?rev=323677&view=rev
Log:
math.h: Set HAVE_HW_FMA32 based on compiler provided macro

Fixes sin/cos piglits on non-FMA capable asics.
Bugzilla: https://bugs.llvm.org/show_bug.cgi?id=35983

Reviewer: Tom Stellard
Signed-off-by: Jan Vesely 

Modified:
libclc/trunk/generic/lib/math/math.h

Modified: libclc/trunk/generic/lib/math/math.h
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/generic/lib/math/math.h?rev=323677&r1=323676&r2=323677&view=diff
==
--- libclc/trunk/generic/lib/math/math.h (original)
+++ libclc/trunk/generic/lib/math/math.h Mon Jan 29 11:05:08 2018
@@ -31,7 +31,12 @@
 #define PNOR 0x100
 #define PINF 0x200
 
+#if (defined __AMDGCN__ | defined __R600__) & !defined __HAS_FMAF__
+#define HAVE_HW_FMA32() (0)
+#else
 #define HAVE_HW_FMA32() (1)
+#endif
+
 #define HAVE_BITALIGN() (0)
 #define HAVE_FAST_FMA32() (0)
 


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

Re: [libclc] r323677 - math.h: Set HAVE_HW_FMA32 based on compiler provided macro

[Roman Lebedev via cfe-commits]

On Mon, Jan 29, 2018 at 10:05 PM, Jan Vesely via cfe-commits
 wrote:
> Author: jvesely
> Date: Mon Jan 29 11:05:08 2018
> New Revision: 323677
>
> URL: http://llvm.org/viewvc/llvm-project?rev=323677&view=rev
> Log:
> math.h: Set HAVE_HW_FMA32 based on compiler provided macro
>
> Fixes sin/cos piglits on non-FMA capable asics.
> Bugzilla: https://bugs.llvm.org/show_bug.cgi?id=35983
>
> Reviewer: Tom Stellard
> Signed-off-by: Jan Vesely 
>
> Modified:
> libclc/trunk/generic/lib/math/math.h
>
> Modified: libclc/trunk/generic/lib/math/math.h
> URL: 
> http://llvm.org/viewvc/llvm-project/libclc/trunk/generic/lib/math/math.h?rev=323677&r1=323676&r2=323677&view=diff
> ==
> --- libclc/trunk/generic/lib/math/math.h (original)
> +++ libclc/trunk/generic/lib/math/math.h Mon Jan 29 11:05:08 2018
> @@ -31,7 +31,12 @@
>  #define PNOR 0x100
>  #define PINF 0x200
>
> +#if (defined __AMDGCN__ | defined __R600__) & !defined __HAS_FMAF__
Shouldn't that be || and && ?

> +#define HAVE_HW_FMA32() (0)
> +#else
>  #define HAVE_HW_FMA32() (1)
> +#endif
> +
>  #define HAVE_BITALIGN() (0)
>  #define HAVE_FAST_FMA32() (0)
>
>
>
> ___
> 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

[PATCH] D42651: [clang-format] Disable some text proto delimiters and functions for google style

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir created this revision.
krasimir added a reviewer: sammccall.
Herald added subscribers: cfe-commits, klimek.

This disables some of the most commonly used text proto delimiters and functions
for google style until we resolve several style options for that style.
In particular, wheter there should be a space surrounding braces ``msg { sub { 
key : value } }``
and the extent of packing of submessages on a same line.


Repository:
  rC Clang

https://reviews.llvm.org/D42651

Files:
  lib/Format/Format.cpp


Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -709,17 +709,9 @@
   {
   "pb",
   "PB",
-  "proto",
-  "PROTO",
-  "textproto",
-  "TEXTPROTO",
   },
   /*EnclosingFunctionNames=*/
-   {
-   "EqualsProto",
-   "PARSE_TEXT_PROTO",
-   "ParseTextProto",
-   },
+  {},
   /*CanonicalDelimiter=*/"",
   /*BasedOnStyle=*/"google",
   }};


Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -709,17 +709,9 @@
   {
   "pb",
   "PB",
-  "proto",
-  "PROTO",
-  "textproto",
-  "TEXTPROTO",
   },
   /*EnclosingFunctionNames=*/
-   {
-   "EqualsProto",
-   "PARSE_TEXT_PROTO",
-   "ParseTextProto",
-   },
+  {},
   /*CanonicalDelimiter=*/"",
   /*BasedOnStyle=*/"google",
   }};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r323678 - [clang-format] Disable some text proto delimiters and functions for google style

[Krasimir Georgiev via cfe-commits]

Author: krasimir
Date: Mon Jan 29 11:28:05 2018
New Revision: 323678

URL: http://llvm.org/viewvc/llvm-project?rev=323678&view=rev
Log:
[clang-format] Disable some text proto delimiters and functions for google style

Summary:
This disables some of the most commonly used text proto delimiters and functions
for google style until we resolve several style options for that style.
In particular, wheter there should be a space surrounding braces ``msg { sub { 
key : value } }``
and the extent of packing of submessages on a same line.

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: klimek, cfe-commits

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

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

Modified: cfe/trunk/lib/Format/Format.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=323678&r1=323677&r2=323678&view=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Mon Jan 29 11:28:05 2018
@@ -709,17 +709,9 @@ FormatStyle getGoogleStyle(FormatStyle::
   {
   "pb",
   "PB",
-  "proto",
-  "PROTO",
-  "textproto",
-  "TEXTPROTO",
   },
   /*EnclosingFunctionNames=*/
-   {
-   "EqualsProto",
-   "PARSE_TEXT_PROTO",
-   "ParseTextProto",
-   },
+  {},
   /*CanonicalDelimiter=*/"",
   /*BasedOnStyle=*/"google",
   }};


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

[PATCH] D42651: [clang-format] Disable some text proto delimiters and functions for google style

[Krasimir Georgiev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL323678: [clang-format] Disable some text proto delimiters 
and functions for google style (authored by krasimir, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D42651

Files:
  cfe/trunk/lib/Format/Format.cpp


Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -709,17 +709,9 @@
   {
   "pb",
   "PB",
-  "proto",
-  "PROTO",
-  "textproto",
-  "TEXTPROTO",
   },
   /*EnclosingFunctionNames=*/
-   {
-   "EqualsProto",
-   "PARSE_TEXT_PROTO",
-   "ParseTextProto",
-   },
+  {},
   /*CanonicalDelimiter=*/"",
   /*BasedOnStyle=*/"google",
   }};


Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -709,17 +709,9 @@
   {
   "pb",
   "PB",
-  "proto",
-  "PROTO",
-  "textproto",
-  "TEXTPROTO",
   },
   /*EnclosingFunctionNames=*/
-   {
-   "EqualsProto",
-   "PARSE_TEXT_PROTO",
-   "ParseTextProto",
-   },
+  {},
   /*CanonicalDelimiter=*/"",
   /*BasedOnStyle=*/"google",
   }};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r323679 - [NFC] Fixup comment with function name, actually incorrect name!

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Mon Jan 29 11:33:20 2018
New Revision: 323679

URL: http://llvm.org/viewvc/llvm-project?rev=323679&view=rev
Log:
[NFC] Fixup comment with function name, actually incorrect name!

Modified:
cfe/trunk/lib/Sema/SemaOverload.cpp

Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=323679&r1=323678&r2=323679&view=diff
==
--- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOverload.cpp Mon Jan 29 11:33:20 2018
@@ -10534,9 +10534,8 @@ static void CompleteNonViableCandidate(S
   }
 }
 
-/// PrintOverloadCandidates - When overload resolution fails, prints
-/// diagnostic messages containing the candidates in the candidate
-/// set.
+/// When overload resolution fails, prints diagnostic messages containing the
+/// candidates in the candidate set.
 void OverloadCandidateSet::NoteCandidates(
 Sema &S, OverloadCandidateDisplayKind OCD, ArrayRef Args,
 StringRef Opc, SourceLocation OpLoc,


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

[PATCH] D42408: [clang-format] Align preprocessor comments with #

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added inline comments.



Comment at: unittests/Format/FormatTest.cpp:2613
Style));
-  // FIXME: The comment indent corrector in TokenAnnotator gets thrown off by
-  // preprocessor indentation.
-  EXPECT_EQ("#if 1\n"
-"  // comment\n"
-"#  define A 0\n"
-"// comment\n"
-"#  define B 0\n"
-"#endif",
-format("#if 1\n"
-   "// comment\n"
-   "#  define A 0\n"
-   "   // comment\n"
-   "#  define B 0\n"
-   "#endif",
-   Style));
+  // Keep comments aligned with #, otherwise indnet comments normally. These
+  // tests cannot use verifyFormat because messUp manipulates leading

s/indnet/indent


Repository:
  rC Clang

https://reviews.llvm.org/D42408



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

Re: [clang-tools-extra] r323149 - [clangd] Drop ~destructor completions - rarely helpful and work inconsistently

[David Blaikie via cfe-commits]

Fair - maybe just a comment in the test case (and/or the source code that
handles this) saying "hey, let's make sure dtors aren't here - but if/when
there's support for a low priority completion group, these should appear
there" or the like.

On Mon, Jan 29, 2018 at 9:49 AM Sam McCall  wrote:

> On Mon, Jan 29, 2018 at 5:31 PM, David Blaikie  wrote:
>
>> Any chance of making this a really low priority completion?
>>
> (Just for clarity - this is a postfilter in clangd only, we haven't
> changed the behavior of the Sema or libclang APIs)
>
> We certainly have considered downranking such items, maybe it's the right
> thing, but it's not without issues on the UX side:
>  - It's far from the only candidate, other things that people want "at the
> bottom" include inaccessible and unavailable members, injected type names,
> operators... it gets crowded, so you still have to decide how to rank them.
>  - LSP doesn't (currently) have any affordance to "grey out" results and
> show users "this is the last good result, the ones below here are unlikely
> to be useful". So the important end-of-list signal is lost.
> There's also a bit of implementation complexity in having
> second/third/fourth "tiers" of results - it adds invariants to the scoring
> logic that make it harder to understand and modify.
>
> (maybe just leaving in a FIXME or expected-fail check of some kind if it's
>> not practical to implement it right now) For those handful of times when
>> you actually want to do this.
>>
> re: practical to implement, there are a few problems with it, apart from
> being rarely useful:
>  - It completes after "foo.", but not "foo.~F", I guess because the parser
> is in the wrong state
>  - it completes ~basic_string, but not ~string
>
> We do indirectly test that destructor completions are turned off in
> clangd. I don't really know where to add the FIXMEs. I can add XFAIL tests
> to c-index-test I think, is anyone likely to go looking for them?
>
> On Mon, Jan 22, 2018 at 1:06 PM Sam McCall via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: sammccall
>>> Date: Mon Jan 22 13:05:00 2018
>>> New Revision: 323149
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=323149&view=rev
>>> Log:
>>> [clangd] Drop ~destructor completions - rarely helpful and work
>>> inconsistently
>>>
>>> Modified:
>>> clang-tools-extra/trunk/clangd/CodeComplete.cpp
>>> clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
>>>
>>> Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=323149&r1=323148&r2=323149&view=diff
>>>
>>> ==
>>> --- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
>>> +++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Mon Jan 22 13:05:00
>>> 2018
>>> @@ -361,6 +361,10 @@ struct CompletionRecorder : public CodeC
>>>(Result.Availability == CXAvailability_NotAvailable ||
>>> Result.Availability == CXAvailability_NotAccessible))
>>>  continue;
>>> +  // Destructor completion is rarely useful, and works
>>> inconsistently.
>>> +  // (s.^ completes ~string, but s.~st^ is an error).
>>> +  if (dyn_cast_or_null(Result.Declaration))
>>> +continue;
>>>Results.push_back(Result);
>>>  }
>>>}
>>>
>>> Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=323149&r1=323148&r2=323149&view=diff
>>>
>>> ==
>>> --- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
>>> (original)
>>> +++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Mon
>>> Jan 22 13:05:00 2018
>>> @@ -461,7 +461,7 @@ TEST(CompletionTest, NoDuplicates) {
>>>{cls("Adapter")});
>>>
>>>// Make sure there are no duplicate entries of 'Adapter'.
>>> -  EXPECT_THAT(Results.items, ElementsAre(Named("Adapter"),
>>> Named("~Adapter")));
>>> +  EXPECT_THAT(Results.items, ElementsAre(Named("Adapter")));
>>>  }
>>>
>>>  TEST(CompletionTest, ScopedNoIndex) {
>>>
>>>
>>> ___
>>> 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

LLVM buildmaster will be updated and restarted tonight

[Galina Kistanova via cfe-commits]

 Hello everyone,

LLVM buildmaster will be updated and restarted after 6 PM Pacific time
today.

Thanks

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

[PATCH] D42641: [MinGW] Emit typeinfo locally for dllimported classes without key functions

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added inline comments.



Comment at: lib/CodeGen/CGVTables.cpp:887-888
 
+  if (CGM.getTriple().isWindowsGNUEnvironment() && 
RD->hasAttr())
+return true;
+

majnemer wrote:
> Maybe a comment like "VTables of classes declared as dllimport are always 
> considered to be external." ?
Ok, I can add that. I wasn't quite sure what to say, since I only ended up here 
in trying to get ShouldUseExternalRTTIDescriptor in ItaniumCXXABI to return 
false. Alternatively I could just add an "if (GNU && DLLImport) return false;" 
in that function instead, if that'd cause less collateral changes (like the 
vtable in dllimport.cpp that changes from available_externally to external).



Comment at: lib/CodeGen/CGVTables.cpp:896-900
   // Otherwise, if the class is an instantiated template, the
   // vtable must be defined here.
   if (TSK == TSK_ImplicitInstantiation ||
   TSK == TSK_ExplicitInstantiationDefinition)
 return false;

majnemer wrote:
> It would be good to have tests for what would have happened if these paths 
> got hit.
Ok - can you hint on what kind of constructs and setups are needed to get here?


https://reviews.llvm.org/D42641



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

r323684 - [clang-format] Add more tests for ObjC protocol list formatting behavior

[Ben Hamilton via cfe-commits]

Author: benhamilton
Date: Mon Jan 29 12:01:49 2018
New Revision: 323684

URL: http://llvm.org/viewvc/llvm-project?rev=323684&view=rev
Log:
[clang-format] Add more tests for ObjC protocol list formatting behavior

Summary:
The existing unit tests in FormatTestObjC.cpp didn't fully cover
all the cases for protocol confirmance list formatting.

This extends the unit tests to more cases of protocol
conformance list formatting, especially how the behavior changes
when `BinPackParameters` changes from `true` (the default) to `false`.

Test Plan: make -j12 FormatTests && \
  ./tools/clang/unittests/Format/FormatTests --gtest_filter=FormatTestObjC.\*

Reviewers: krasimir, jolesiak, stephanemoore

Reviewed By: krasimir

Subscribers: benhamilton, klimek, cfe-commits, hokein, Wizard

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

Modified:
cfe/trunk/unittests/Format/FormatTestObjC.cpp

Modified: cfe/trunk/unittests/Format/FormatTestObjC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestObjC.cpp?rev=323684&r1=323683&r2=323684&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp Mon Jan 29 12:01:49 2018
@@ -276,6 +276,20 @@ TEST_F(FormatTestObjC, FormatObjCInterfa
"+ (id)init;\n"
"@end");
 
+  Style.ColumnLimit = 40;
+  verifyFormat("@interface c () <\n"
+   "c, c,\n"
+   "c, c> {\n"
+   "}");
+
+  Style.BinPackParameters = false;
+  verifyFormat("@interface d () <\n"
+   "d,\n"
+   "d,\n"
+   "d,\n"
+   "d> {\n"
+   "}");
+
   Style = getGoogleStyle(FormatStyle::LK_ObjC);
   verifyFormat("@interface Foo : NSObject  {\n"
" @public\n"


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

[PATCH] D42642: [CUDA] Detect installation in PATH

[Artem Belevich via Phabricator via cfe-commits]

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

Some linux distributions integrate CUDA into the standard directory structure. 
I.e. binaries go into /usr/bin, headers into /usr/include, bitcode goes 
somewhere else, etc. ptxas will be found, but we would still fail to detect 
CUDA. I'd add  one more test case to make sure that's still the case.

Otherwise - LGTM.


Repository:
  rC Clang

https://reviews.llvm.org/D42642



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

[PATCH] D42649: [clang-format] Add more tests for ObjC protocol list formatting behavior

[Ben Hamilton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL323684: [clang-format] Add more tests for ObjC protocol list 
formatting behavior (authored by benhamilton, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D42649

Files:
  cfe/trunk/unittests/Format/FormatTestObjC.cpp


Index: cfe/trunk/unittests/Format/FormatTestObjC.cpp
===
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp
@@ -276,6 +276,20 @@
"+ (id)init;\n"
"@end");
 
+  Style.ColumnLimit = 40;
+  verifyFormat("@interface c () <\n"
+   "c, c,\n"
+   "c, c> {\n"
+   "}");
+
+  Style.BinPackParameters = false;
+  verifyFormat("@interface d () <\n"
+   "d,\n"
+   "d,\n"
+   "d,\n"
+   "d> {\n"
+   "}");
+
   Style = getGoogleStyle(FormatStyle::LK_ObjC);
   verifyFormat("@interface Foo : NSObject  {\n"
" @public\n"


Index: cfe/trunk/unittests/Format/FormatTestObjC.cpp
===
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp
@@ -276,6 +276,20 @@
"+ (id)init;\n"
"@end");
 
+  Style.ColumnLimit = 40;
+  verifyFormat("@interface c () <\n"
+   "c, c,\n"
+   "c, c> {\n"
+   "}");
+
+  Style.BinPackParameters = false;
+  verifyFormat("@interface d () <\n"
+   "d,\n"
+   "d,\n"
+   "d,\n"
+   "d> {\n"
+   "}");
+
   Style = getGoogleStyle(FormatStyle::LK_ObjC);
   verifyFormat("@interface Foo : NSObject  {\n"
" @public\n"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42649: [clang-format] Add more tests for ObjC protocol list formatting behavior

[Ben Hamilton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC323684: [clang-format] Add more tests for ObjC protocol list 
formatting behavior (authored by benhamilton, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D42649?vs=131821&id=131840#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D42649

Files:
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -276,6 +276,20 @@
"+ (id)init;\n"
"@end");
 
+  Style.ColumnLimit = 40;
+  verifyFormat("@interface c () <\n"
+   "c, c,\n"
+   "c, c> {\n"
+   "}");
+
+  Style.BinPackParameters = false;
+  verifyFormat("@interface d () <\n"
+   "d,\n"
+   "d,\n"
+   "d,\n"
+   "d> {\n"
+   "}");
+
   Style = getGoogleStyle(FormatStyle::LK_ObjC);
   verifyFormat("@interface Foo : NSObject  {\n"
" @public\n"


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -276,6 +276,20 @@
"+ (id)init;\n"
"@end");
 
+  Style.ColumnLimit = 40;
+  verifyFormat("@interface c () <\n"
+   "c, c,\n"
+   "c, c> {\n"
+   "}");
+
+  Style.BinPackParameters = false;
+  verifyFormat("@interface d () <\n"
+   "d,\n"
+   "d,\n"
+   "d,\n"
+   "d> {\n"
+   "}");
+
   Style = getGoogleStyle(FormatStyle::LK_ObjC);
   verifyFormat("@interface Foo : NSObject  {\n"
" @public\n"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42650: [clang-format] New format param BinPackObjCProtocolList

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton updated this revision to Diff 131841.
benhamilton added a comment.

Rebase


Repository:
  rC Clang

https://reviews.llvm.org/D42650

Files:
  include/clang/Format/Format.h
  lib/Format/ContinuationIndenter.cpp
  lib/Format/Format.cpp
  unittests/Format/FormatTestObjC.cpp

Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -281,15 +281,28 @@
"c, c,\n"
"c, c> {\n"
"}");
-
-  Style.BinPackParameters = false;
+  Style.BinPackObjCProtocolList = FormatStyle::BPS_Never;
   verifyFormat("@interface d () <\n"
"d,\n"
"d,\n"
"d,\n"
"d> {\n"
"}");
 
+  Style.BinPackParameters = false;
+  Style.BinPackObjCProtocolList = FormatStyle::BPS_Auto;
+  verifyFormat("@interface e () <\n"
+   "e,\n"
+   "e,\n"
+   "e,\n"
+   "e> {\n"
+   "}");
+  Style.BinPackObjCProtocolList = FormatStyle::BPS_Always;
+  verifyFormat("@interface f () <\n"
+   "f, f,\n"
+   "f, f> {\n"
+   "}");
+
   Style = getGoogleStyle(FormatStyle::LK_ObjC);
   verifyFormat("@interface Foo : NSObject  {\n"
" @public\n"
@@ -309,13 +322,16 @@
   verifyFormat("@interface Foo (HackStuff) \n"
"+ (id)init;\n"
"@end");
-  Style.BinPackParameters = false;
-  Style.ColumnLimit = 80;
-  verifyFormat("@interface a () <\n"
-   "a,\n"
-   ",\n"
-   "aa,\n"
-   "> {\n"
+  Style.ColumnLimit = 40;
+  // BinPackParameters should be true by default.
+  verifyFormat("void (int e, int e,\n"
+   "  int e, int e);\n");
+  // BinPackObjCProtocolList should be BPS_Never by default.
+  verifyFormat("@interface f () <\n"
+   "f,\n"
+   "f,\n"
+   "f,\n"
+   "f> {\n"
"}");
 }
 
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -105,6 +105,14 @@
   }
 };
 
+template <> struct ScalarEnumerationTraits {
+  static void enumeration(IO &IO, FormatStyle::BinPackStyle &Value) {
+IO.enumCase(Value, "Auto", FormatStyle::BPS_Auto);
+IO.enumCase(Value, "Always", FormatStyle::BPS_Always);
+IO.enumCase(Value, "Never", FormatStyle::BPS_Never);
+  }
+};
+
 template <> struct ScalarEnumerationTraits {
   static void enumeration(IO &IO, FormatStyle::BinaryOperatorStyle &Value) {
 IO.enumCase(Value, "All", FormatStyle::BOS_All);
@@ -323,6 +331,7 @@
Style.AlwaysBreakTemplateDeclarations);
 IO.mapOptional("BinPackArguments", Style.BinPackArguments);
 IO.mapOptional("BinPackParameters", Style.BinPackParameters);
+IO.mapOptional("BinPackObjCProtocolList", Style.BinPackObjCProtocolList);
 IO.mapOptional("BraceWrapping", Style.BraceWrapping);
 IO.mapOptional("BreakBeforeBinaryOperators",
Style.BreakBeforeBinaryOperators);
@@ -599,6 +608,7 @@
   LLVMStyle.AlwaysBreakTemplateDeclarations = false;
   LLVMStyle.BinPackArguments = true;
   LLVMStyle.BinPackParameters = true;
+  LLVMStyle.BinPackObjCProtocolList = FormatStyle::BPS_Auto;
   LLVMStyle.BreakBeforeBinaryOperators = FormatStyle::BOS_None;
   LLVMStyle.BreakBeforeTernaryOperators = true;
   LLVMStyle.BreakBeforeBraces = FormatStyle::BS_Attach;
@@ -752,6 +762,7 @@
 GoogleStyle.SpacesInContainerLiterals = false;
   } else if (Language == FormatStyle::LK_ObjC) {
 GoogleStyle.ColumnLimit = 100;
+GoogleStyle.BinPackObjCProtocolList = FormatStyle::BPS_Never;
   }
 
   return GoogleStyle;
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -1222,9 +1222,20 @@
 Current.MatchingParen->getPreviousNonComment() &&
 Current.MatchingParen->getPreviousNonComment()->is(tok::comma);
 
+// If BinPackObjCProtocolList is unspecified, fall back to BinPackParameters
+// for backwards compatibility.
+bool BinPackObjCProtocolList =
+(Style.BinPackObjCProtocolList == FormatStyle::BPS_Auto &&
+ Style.BinPack

[PATCH] D42642: [CUDA] Detect installation in PATH

[Justin Lebar via Phabricator via cfe-commits]

jlebar added a comment.

Can we document this behavior in https://llvm.org/docs/CompileCudaWithLLVM.html 
(in the LLVM repo)?  Totally fine if you want to do this in a separate patch.


Repository:
  rC Clang

https://reviews.llvm.org/D42642



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

[PATCH] D41553: Support parsing double square-bracket attributes in ObjC

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 131860.
aaron.ballman added a comment.

Updates based on review feedback.


https://reviews.llvm.org/D41553

Files:
  include/clang/Basic/Attr.td
  include/clang/Parse/Parser.h
  lib/Parse/ParseObjc.cpp
  lib/Parse/Parser.cpp
  test/Misc/ast-dump-attr.m
  test/Parser/objc-attr.m

Index: test/Parser/objc-attr.m
===
--- test/Parser/objc-attr.m
+++ test/Parser/objc-attr.m
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -fsyntax-only -fdouble-square-bracket-attributes -triple x86_64-apple-macosx10.10.0 -verify %s
+// expected-no-diagnostics
+
+@interface NSObject
+@end
+
+[[clang::objc_exception]]
+@interface Foo {
+  [[clang::iboutlet]] NSObject *h;
+}
+@property (readonly) [[clang::objc_returns_inner_pointer]] void *i, *j;
+@property (readonly) [[clang::iboutlet]] NSObject *k;
+@end
+
+[[clang::objc_runtime_name("name")]] @protocol Bar;
+
+[[clang::objc_protocol_requires_explicit_implementation]] 
+@protocol Baz
+@end
+
+@interface Quux
+-(void)g1 [[clang::ns_consumes_self]];
+-(void)g2 __attribute__((ns_consumes_self));
+-(void)h1: (int)x [[clang::ns_consumes_self]];
+-(void)h2: (int)x __attribute__((ns_consumes_self));
+-(void) [[clang::ns_consumes_self]] i1;
+-(void) __attribute__((ns_consumes_self)) i2;
+@end
Index: test/Misc/ast-dump-attr.m
===
--- test/Misc/ast-dump-attr.m
+++ test/Misc/ast-dump-attr.m
@@ -0,0 +1,57 @@
+// RUN: %clang_cc1 -fdouble-square-bracket-attributes -triple x86_64-apple-macosx10.10.0 -ast-dump -ast-dump-filter Test %s | FileCheck --strict-whitespace %s
+
+@interface NSObject
+@end
+
+[[clang::objc_exception]]
+@interface Test1 {
+// CHECK: ObjCInterfaceDecl{{.*}} Test1
+// CHECK-NEXT: ObjCExceptionAttr{{.*}}
+  [[clang::iboutlet]] NSObject *Test2;
+// CHECK: ObjCIvarDecl{{.*}} Test2
+// CHECK-NEXT: IBOutletAttr
+}
+@property (readonly) [[clang::objc_returns_inner_pointer]] void *Test3, *Test4;
+// CHECK: ObjCPropertyDecl{{.*}} Test3 'void *' readonly
+// CHECK-NEXT: ObjCReturnsInnerPointerAttr
+// CHECK-NEXT: ObjCPropertyDecl{{.*}} Test4 'void *' readonly
+// CHECK-NEXT: ObjCReturnsInnerPointerAttr
+
+@property (readonly) [[clang::iboutlet]] NSObject *Test5;
+// CHECK: ObjCPropertyDecl{{.*}} Test5 'NSObject *' readonly
+// CHECK-NEXT: IBOutletAttr
+
+// CHECK: ObjCMethodDecl{{.*}} implicit{{.*}} Test3
+// CHECK-NEXT: ObjCReturnsInnerPointerAttr
+// CHECK: ObjCMethodDecl{{.*}} implicit{{.*}} Test4
+// CHECK-NEXT: ObjCReturnsInnerPointerAttr
+// CHECK: ObjCMethodDecl{{.*}} implicit{{.*}} Test5
+// CHECK-NOT: IBOutletAttr
+@end
+
+[[clang::objc_runtime_name("name")]] @protocol Test6;
+// CHECK: ObjCProtocolDecl{{.*}} Test6
+// CHECK-NEXT: ObjCRuntimeNameAttr{{.*}} "name"
+
+[[clang::objc_protocol_requires_explicit_implementation]]
+@protocol Test7
+// CHECK: ObjCProtocolDecl{{.*}} Test7
+// CHECK-NEXT: ObjCExplicitProtocolImplAttr
+@end
+
+@interface Test8
+// CHECK: ObjCInterfaceDecl{{.*}} Test8
+-(void)Test9 [[clang::ns_consumes_self]];
+// CHECK: ObjCMethodDecl{{.*}} Test9 'void'
+// CHECK-NEXT: NSConsumesSelfAttr
+-(void) [[clang::ns_consumes_self]] Test10: (int)Test11;
+// CHECK: ObjCMethodDecl{{.*}} Test10: 'void'
+// CHECK-NEXT: |-ParmVarDecl{{.*}} Test11 'int'
+// CHECK-NEXT: `-NSConsumesSelfAttr
+-(void)Test12: (int *) [[clang::noescape]] Test13  to:(int)Test14 [[clang::ns_consumes_self]];
+// CHECK: ObjCMethodDecl{{.*}} Test12:to: 'void'
+// CHECK-NEXT: |-ParmVarDecl{{.*}} Test13 'int *'
+// CHECK-NEXT: | `-NoEscapeAttr
+// CHECK-NEXT: |-ParmVarDecl{{.*}} Test14 'int'
+// CHECK-NEXT: `-NSConsumesSelfAttr
+@end
Index: lib/Parse/Parser.cpp
===
--- lib/Parse/Parser.cpp
+++ lib/Parse/Parser.cpp
@@ -741,7 +741,7 @@
 break;
   }
   case tok::at:
-return ParseObjCAtDirectives();
+return ParseObjCAtDirectives(attrs);
   case tok::minus:
   case tok::plus:
 if (!getLangOpts().ObjC1) {
Index: lib/Parse/ParseObjc.cpp
===
--- lib/Parse/ParseObjc.cpp
+++ lib/Parse/ParseObjc.cpp
@@ -45,7 +45,8 @@
 /// [OBJC]  objc-protocol-definition
 /// [OBJC]  objc-method-definition
 /// [OBJC]  '@' 'end'
-Parser::DeclGroupPtrTy Parser::ParseObjCAtDirectives() {
+Parser::DeclGroupPtrTy
+Parser::ParseObjCAtDirectives(ParsedAttributesWithRange &Attrs) {
   SourceLocation AtLoc = ConsumeToken(); // the "@"
 
   if (Tok.is(tok::code_completion)) {
@@ -58,15 +59,11 @@
   switch (Tok.getObjCKeywordID()) {
   case tok::objc_class:
 return ParseObjCAtClassDeclaration(AtLoc);
-  case tok::objc_interface: {
-ParsedAttributes attrs(AttrFactory);
-SingleDecl = ParseObjCAtInterfaceDeclaration(AtLoc, attrs);
+  case tok::objc_interface:
+SingleDecl = ParseObjCAtInterfaceDeclaration(AtLoc, Attrs);
 break;
-  }
-  case tok::objc_protocol: {
-ParsedAttributes attrs(AttrFactory);
-return Pa

[PATCH] D41553: Support parsing double square-bracket attributes in ObjC

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman marked 5 inline comments as done.
aaron.ballman added inline comments.



Comment at: lib/Parse/ParseObjc.cpp:1434
   MaybeParseGNUAttributes(paramAttrs);
-  ArgInfo.ArgAttrs = paramAttrs.getList();
 }
 

rjmccall wrote:
> ObjC parameter syntax is really its own weird thing.  I think this is the 
> right place to allow attributes, after the parenthesized type but before the 
> parameter-variable name (which is optional).  And of course we also allow 
> them within the parentheses, but hopefully that just falls out from parsing 
> the type.
Within the parens is tricky -- if the attribute appears immediately after the 
type, then it appertain to the *type* and not the *declaration*.
```
-(void)Test: (int) [[foo]] i  to:(int [[bar]]) j from: ([[baz]] int) k;
```
I would expect `foo` to appertain to `i`, `bar` to appertain to the type `int`, 
and `baz` to be ill-formed, which is what will fall out from parsing the type.


https://reviews.llvm.org/D41553



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

[libcxx] r323694 - Minor updated to the main libcxx page; add a link to the deprecation page

[Marshall Clow via cfe-commits]

Author: marshall
Date: Mon Jan 29 13:28:46 2018
New Revision: 323694

URL: http://llvm.org/viewvc/llvm-project?rev=323694&view=rev
Log:
Minor updated to the main libcxx page; add a link to the deprecation page

Modified:
libcxx/trunk/www/index.html

Modified: libcxx/trunk/www/index.html
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/www/index.html?rev=323694&r1=323693&r2=323694&view=diff
==
--- libcxx/trunk/www/index.html (original)
+++ libcxx/trunk/www/index.html Mon Jan 29 13:28:46 2018
@@ -134,14 +134,17 @@
 
libc++ is a 100% complete C++11 implementation on Apple's OS X. 
LLVM and Clang can self host in C++ and C++11 mode with libc++ on 
Linux.
-   libc++ is also a 100% complete C++14 implementation. A list of new 
features and changes for
-  C++14 can be found here.
+   libc++ is also a 100% complete C++14 implementation. A list of new 
features and
+  changes for C++14 can be found here.
+   libc++'s C++17 implementation is not yet complete. A list of features 
and changes
+  for C++17 can be found here.
A list of features and changes for the next C++ standard, known here as
-  "C++1z" (probably to be C++17) can be found here.
-   A list of features and changes for the C++ standard beyond C++17, known 
here as
   "C++2a" (probably to be C++20) can be found here.
-   Implementation of the post-c++14 Technical Specifications is in 
progress. A list of features and
-  the current status of these features can be found here.
+   Implementation of the post-C++14 Technical Specifications is in 
progress. A list of features
+  and the current status of these features can be found here.
+   As features get moved from the Technical Specifications into the main 
standard, we
+  will (after a period for migration) remove them from the TS 
implementation. This
+  process is detailed here.
 


Build Bots


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

[PATCH] D42642: [CUDA] Detect installation in PATH

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

In https://reviews.llvm.org/D42642#990976, @tra wrote:

> Some linux distributions integrate CUDA into the standard directory 
> structure. I.e. binaries go into /usr/bin, headers into /usr/include, bitcode 
> goes somewhere else, etc. ptxas will be found, but we would still fail to 
> detect CUDA. I'd add  one more test case to make sure that's still the case.


I'm not sure how this can work, we only require `bin/` and `include/` to exist, 
and `nvvm/libdevice/` if `-nocudalib` isn't specified. I agree this can be a 
problem because the defaults might detect an invalid installation...

In https://reviews.llvm.org/D42642#991013, @jlebar wrote:

> Can we document this behavior in 
> https://llvm.org/docs/CompileCudaWithLLVM.html (in the LLVM repo)?  Totally 
> fine if you want to do this in a separate patch.


Sure, will do!


Repository:
  rC Clang

https://reviews.llvm.org/D42642



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

[PATCH] D42464: add prefix with '_' support for property name. Corresponding apple dev doc: https://developer.apple.com/library/content/qa/qa1908/_index.html

[Yan Zhang via Phabricator via cfe-commits]

Wizard updated this revision to Diff 131869.
Wizard added a comment.

resolve comments and fix some logic


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42464

Files:
  clang-tidy/objc/PropertyDeclarationCheck.cpp
  docs/clang-tidy/checks/objc-property-declaration.rst
  test/clang-tidy/objc-property-declaration-custom.m
  test/clang-tidy/objc-property-declaration.m

Index: test/clang-tidy/objc-property-declaration.m
===
--- test/clang-tidy/objc-property-declaration.m
+++ test/clang-tidy/objc-property-declaration.m
@@ -3,11 +3,29 @@
 
 @interface Foo
 @property(assign, nonatomic) int NotCamelCase;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'NotCamelCase' should use lowerCamelCase style, according to the Apple Coding Guidelines [objc-property-declaration]
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'NotCamelCase' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
 // CHECK-FIXES: @property(assign, nonatomic) int notCamelCase;
 @property(assign, nonatomic) int camelCase;
 @property(strong, nonatomic) NSString *URLString;
 @property(strong, nonatomic) NSString *bundleID;
 @property(strong, nonatomic) NSString *URL_string;
-// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'URL_string' should use lowerCamelCase style, according to the Apple Coding Guidelines [objc-property-declaration]
+// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'URL_string' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
 @end
+
+@interface Foo (Bar)
+@property(assign, nonatomic) int abc_NotCamelCase;
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'abc_NotCamelCase' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
+@property(assign, nonatomic) int abCD_camelCase;
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'abCD_camelCase' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
+// CHECK-FIXES: @property(assign, nonatomic) int abcd_camelCase;
+@property(assign, nonatomic) int abCD_NotCamelCase;
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'abCD_NotCamelCase' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
+// CHECK-FIXES: @property(assign, nonatomic) int abcd_notCamelCase;
+@property(strong, nonatomic) NSString *URLStr;
+@property(assign, nonatomic) int abc_camelCase;
+@end
+
+@interface Foo ()
+@property(assign, nonatomic) int abc_inClassExtension;
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'abc_inClassExtension' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
+@end
\ No newline at end of file
Index: test/clang-tidy/objc-property-declaration-custom.m
===
--- test/clang-tidy/objc-property-declaration-custom.m
+++ test/clang-tidy/objc-property-declaration-custom.m
@@ -6,9 +6,9 @@
 
 @interface Foo
 @property(assign, nonatomic) int AbcNotRealPrefix;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'AbcNotRealPrefix' should use lowerCamelCase style, according to the Apple Coding Guidelines [objc-property-declaration]
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'AbcNotRealPrefix' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
 // CHECK-FIXES: @property(assign, nonatomic) int abcNotRealPrefix;
 @property(assign, nonatomic) int ABCCustomPrefix;
 @property(strong, nonatomic) NSString *ABC_custom_prefix;
-// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'ABC_custom_prefix' should use lowerCamelCase style, according to the Apple Coding Guidelines [objc-property-declaration]
+// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'ABC_custom_prefix' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
 @end
Index: docs/clang-tidy/checks/objc-property-declaration.rst
===
--- docs/clang-tidy/checks/objc-property-declaration.rst
+++ docs/clang-tidy/checks/objc-property-declaration.rst
@@ -32,6 +32,15 @@
 
 The corresponding style rule: https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/CodingGuidelines/Articles/NamingIvarsAndTypes.html#//apple_ref/doc/uid/20001284-1001757
 
+The check will also accept property declared in category with a prefix of
+lowercase letters followed by a '_' to avoid naming conflict. For example:
+
+.. code-

[PATCH] D42642: [CUDA] Detect installation in PATH

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In https://reviews.llvm.org/D42642#991127, @Hahnfeld wrote:

> In https://reviews.llvm.org/D42642#990976, @tra wrote:
>
> > Some linux distributions integrate CUDA into the standard directory 
> > structure. I.e. binaries go into /usr/bin, headers into /usr/include, 
> > bitcode goes somewhere else, etc. ptxas will be found, but we would still 
> > fail to detect CUDA. I'd add  one more test case to make sure that's still 
> > the case.
>
>
> I'm not sure how this can work, we only require `bin/` and `include/` to 
> exist, and `nvvm/libdevice/` if `-nocudalib` isn't specified. I agree this 
> can be a problem because the defaults might detect an invalid installation...


Good point. Perhaps we want to be more strict about CUDA installation checking 
if we've found it indirectly via PATH (as opposed to explicit --cuda-path or a 
known common install path).
Should we always check for nvvm/libdevice directory? It's unlikely to be under 
/usr or /usr/local and it will be always present in a CUDA installation of all 
currently supported CUDA versions.


Repository:
  rC Clang

https://reviews.llvm.org/D42642



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

r323696 - [analyzer] [NFC] Remove unused method visitItemsInWorkList

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Mon Jan 29 13:44:49 2018
New Revision: 323696

URL: http://llvm.org/viewvc/llvm-project?rev=323696&view=rev
Log:
[analyzer] [NFC] Remove unused method visitItemsInWorkList

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

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/WorkList.h
cfe/trunk/lib/StaticAnalyzer/Core/CoreEngine.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/WorkList.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/WorkList.h?rev=323696&r1=323695&r2=323696&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/WorkList.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/WorkList.h Mon 
Jan 29 13:44:49 2018
@@ -80,14 +80,6 @@ public:
   void setBlockCounter(BlockCounter C) { CurrentCounter = C; }
   BlockCounter getBlockCounter() const { return CurrentCounter; }
 
-  class Visitor {
-  public:
-Visitor() {}
-virtual ~Visitor();
-virtual bool visit(const WorkListUnit &U) = 0;
-  };
-  virtual bool visitItemsInWorkList(Visitor &V) = 0;
-  
   static WorkList *makeDFS();
   static WorkList *makeBFS();
   static WorkList *makeBFSBlockDFSContents();

Modified: cfe/trunk/lib/StaticAnalyzer/Core/CoreEngine.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/CoreEngine.cpp?rev=323696&r1=323695&r2=323696&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/CoreEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/CoreEngine.cpp Mon Jan 29 13:44:49 2018
@@ -37,8 +37,6 @@ STATISTIC(NumPathsExplored,
 // Worklist classes for exploration of reachable states.
 
//===--===//
 
-WorkList::Visitor::~Visitor() {}
-
 namespace {
 class DFS : public WorkList {
   SmallVector Stack;
@@ -57,15 +55,6 @@ public:
 Stack.pop_back(); // This technically "invalidates" U, but we are fine.
 return U;
   }
-
-  bool visitItemsInWorkList(Visitor &V) override {
-for (SmallVectorImpl::iterator
- I = Stack.begin(), E = Stack.end(); I != E; ++I) {
-  if (V.visit(*I))
-return true;
-}
-return false;
-  }
 };
 
 class BFS : public WorkList {
@@ -85,14 +74,6 @@ public:
 return U;
   }
 
-  bool visitItemsInWorkList(Visitor &V) override {
-for (std::deque::iterator
- I = Queue.begin(), E = Queue.end(); I != E; ++I) {
-  if (V.visit(*I))
-return true;
-}
-return false;
-  }
 };
 
 } // end anonymous namespace
@@ -135,20 +116,6 @@ namespace {
   Queue.pop_front();
   return U;
 }
-bool visitItemsInWorkList(Visitor &V) override {
-  for (SmallVectorImpl::iterator
-   I = Stack.begin(), E = Stack.end(); I != E; ++I) {
-if (V.visit(*I))
-  return true;
-  }
-  for (std::deque::iterator
-   I = Queue.begin(), E = Queue.end(); I != E; ++I) {
-if (V.visit(*I))
-  return true;
-  }
-  return false;
-}
-
   };
 } // end anonymous namespace
 


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

r323697 - [analyzer] Use stable filenames in analyzer testing infrastructure

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Mon Jan 29 13:45:07 2018
New Revision: 323697

URL: http://llvm.org/viewvc/llvm-project?rev=323697&view=rev
Log:
[analyzer] Use stable filenames in analyzer testing infrastructure

Makes finding the right file in test results easier.

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

Modified:
cfe/trunk/utils/analyzer/SATestBuild.py

Modified: cfe/trunk/utils/analyzer/SATestBuild.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/analyzer/SATestBuild.py?rev=323697&r1=323696&r2=323697&view=diff
==
--- cfe/trunk/utils/analyzer/SATestBuild.py (original)
+++ cfe/trunk/utils/analyzer/SATestBuild.py Mon Jan 29 13:45:07 2018
@@ -247,6 +247,7 @@ def runScanBuild(Dir, SBOutputDir, PBuil
 SBOptions += "-plist-html -o '%s' " % SBOutputDir
 SBOptions += "-enable-checker " + AllCheckers + " "
 SBOptions += "--keep-empty "
+SBOptions += "-analyzer-config 'stable-report-filename=true' "
 # Always use ccc-analyze to ensure that we can locate the failures
 # directory.
 SBOptions += "--override-compiler "


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

Re: r323485 - [Driver] Add an -fexperimental-isel driver option to enable/disable GlobalISel.

[Amara Emerson via cfe-commits]

Hi Hans,

Can we have this for the 6.0 branch? I'm also going to send a patch to add
the release notes clang and LLVM about this flag and GISel being enabled at
-O0.

Cheers,
Amara

On 25 January 2018 at 16:27, Amara Emerson via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: aemerson
> Date: Thu Jan 25 16:27:22 2018
> New Revision: 323485
>
> URL: http://llvm.org/viewvc/llvm-project?rev=323485&view=rev
> Log:
> [Driver] Add an -fexperimental-isel driver option to enable/disable
> GlobalISel.
>
> Differential Revision: https://reviews.llvm.org/D42276
>
> Added:
> cfe/trunk/test/Driver/global-isel.c
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
> cfe/trunk/include/clang/Basic/DiagnosticGroups.td
> cfe/trunk/include/clang/Driver/Options.td
> cfe/trunk/lib/Driver/ToolChains/Clang.cpp
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/
> DiagnosticDriverKinds.td?rev=323485&r1=323484&r2=323485&view=diff
> 
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Thu Jan 25
> 16:27:22 2018
> @@ -361,4 +361,12 @@ def warn_drv_fine_grained_bitfield_acces
>  def note_drv_verify_prefix_spelling : Note<
>"-verify prefixes must start with a letter and contain only
> alphanumeric"
>" characters, hyphens, and underscores">;
> +
> +def warn_drv_experimental_isel_incomplete : Warning<
> +  "-fexperimental-isel support for the '%0' architecture is incomplete">,
> +  InGroup;
> +
> +def warn_drv_experimental_isel_incomplete_opt : Warning<
> +  "-fexperimental-isel support is incomplete for this architecture at the
> current optimization level">,
> +  InGroup;
>  }
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/Basic/DiagnosticGroups.td?rev=323485&r1=323484&r2=323485&view=diff
> 
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Thu Jan 25 16:27:22
> 2018
> @@ -985,3 +985,6 @@ def UnknownArgument : DiagGroup<"unknown
>  // A warning group for warnings about code that clang accepts when
>  // compiling OpenCL C/C++ but which is not compatible with the SPIR spec.
>  def SpirCompat : DiagGroup<"spir-compat">;
> +
> +// Warning for the experimental-isel options.
> +def ExperimentalISel : DiagGroup<"experimental-isel">;
>
> Modified: cfe/trunk/include/clang/Driver/Options.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/Driver/Options.td?rev=323485&r1=323484&r2=323485&view=diff
> 
> ==
> --- cfe/trunk/include/clang/Driver/Options.td (original)
> +++ cfe/trunk/include/clang/Driver/Options.td Thu Jan 25 16:27:22 2018
> @@ -1033,6 +1033,8 @@ def finline_functions : Flag<["-"], "fin
>  def finline_hint_functions: Flag<["-"], "finline-hint-functions">,
> Group, Flags<[CC1Option]>,
>HelpText<"Inline functions which are (explicitly or implicitly) marked
> inline">;
>  def finline : Flag<["-"], "finline">, Group;
> +def fexperimental_isel : Flag<["-"], "fexperimental-isel">,
> Group,
> +  HelpText<"Enables the experimental global instruction selector">;
>  def fexperimental_new_pass_manager : Flag<["-"], "fexperimental-new-pass-
> manager">,
>Group, Flags<[CC1Option]>,
>HelpText<"Enables an experimental new pass manager in LLVM.">;
> @@ -1244,6 +1246,8 @@ def fno_exceptions : Flag<["-"], "fno-ex
>  def fno_gnu_keywords : Flag<["-"], "fno-gnu-keywords">, Group,
> Flags<[CC1Option]>;
>  def fno_inline_functions : Flag<["-"], "fno-inline-functions">,
> Group, Flags<[CC1Option]>;
>  def fno_inline : Flag<["-"], "fno-inline">, Group,
> Flags<[CC1Option]>;
> +def fno_experimental_isel : Flag<["-"], "fno-experimental-isel">,
> Group,
> +  HelpText<"Disables the experimental global instruction selector">;
>  def fno_experimental_new_pass_manager : Flag<["-"],
> "fno-experimental-new-pass-manager">,
>Group, Flags<[CC1Option]>,
>HelpText<"Disables an experimental new pass manager in LLVM.">;
>
> Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/
> ToolChains/Clang.cpp?rev=323485&r1=323484&r2=323485&view=diff
> 
> ==
> --- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
> +++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Thu Jan 25 16:27:22 2018
> @@ -4694,6 +4694,37 @@ void Clang::ConstructJob(Compilation &C,
>  CmdArgs.push_back("-fwhole-program-vtables");
>}
>
> +  if (Arg *A = Args.getLastArg(options::OPT_fexperimental_isel,
> + 

Re: [PATCH] [analyzer] Fix -x language argument for C preprocessed sources

[Jon Roelofs via cfe-commits]

r323664

On Mon, Jan 15, 2018 at 8:35 AM, Jonathan Roelofs  wrote:

> LGTM. Would you like me to commit it for you?
>
>
> Jon
>
>
> On 1/14/18 3:50 AM, Paul Fertser wrote:
>
>> clang's -x option doesn't accept c-cpp-output as a language (even though
>> 463eb6ab was merged, the driver still doesn't handle that).
>>
>> This bug prevents testing C language projects when ccache is used.
>>
>> Fixes #25851.
>>
>> Investigation and patch by Dave Rigby.
>> ---
>>   tools/scan-build/libexec/ccc-analyzer | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/scan-build/libexec/ccc-analyzer
>> b/tools/scan-build/libexec/ccc-analyzer
>> index b0ec7e7..73cd2ff 100755
>> --- a/tools/scan-build/libexec/ccc-analyzer
>> +++ b/tools/scan-build/libexec/ccc-analyzer
>> @@ -419,7 +419,7 @@ my %LangMap = (
>> 'cc'  => 'c++',
>> 'C'   => 'c++',
>> 'ii'  => 'c++-cpp-output',
>> -  'i'   => $IsCXX ? 'c++-cpp-output' : 'c-cpp-output',
>> +  'i'   => $IsCXX ? 'c++-cpp-output' : 'cpp-output',
>> 'm'   => 'objective-c',
>> 'mi'  => 'objective-c-cpp-output',
>> 'mm'  => 'objective-c++',
>> @@ -439,7 +439,7 @@ my %LangsAccepted = (
>> "c" => 1,
>> "c++" => 1,
>> "objective-c++" => 1,
>> -  "c-cpp-output" => 1,
>> +  "cpp-output" => 1,
>> "objective-c-cpp-output" => 1,
>> "c++-cpp-output" => 1
>>   );
>>
>
> --
> Jon Roelofs
> jonat...@codesourcery.com
> CodeSourcery / Mentor Embedded / Siemens
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41553: Support parsing double square-bracket attributes in ObjC

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Parse/ParseObjc.cpp:1434
   MaybeParseGNUAttributes(paramAttrs);
-  ArgInfo.ArgAttrs = paramAttrs.getList();
 }
 

aaron.ballman wrote:
> rjmccall wrote:
> > ObjC parameter syntax is really its own weird thing.  I think this is the 
> > right place to allow attributes, after the parenthesized type but before 
> > the parameter-variable name (which is optional).  And of course we also 
> > allow them within the parentheses, but hopefully that just falls out from 
> > parsing the type.
> Within the parens is tricky -- if the attribute appears immediately after the 
> type, then it appertain to the *type* and not the *declaration*.
> ```
> -(void)Test: (int) [[foo]] i  to:(int [[bar]]) j from: ([[baz]] int) k;
> ```
> I would expect `foo` to appertain to `i`, `bar` to appertain to the type 
> `int`, and `baz` to be ill-formed, which is what will fall out from parsing 
> the type.
As you say, the position at the start of a type-specifier-seq normally applies 
to the declared entity and so cannot have attributes in an abstract declarator. 
 The parens production is meant to resemble a cast, which is normally just `'(' 
type-id ')'`, but this is also part a declaration of a variable.  I don't think 
it's unnatural to allow declaration attributes after the l-paren, and in 
practice that's a common place where ObjC programmers write parameter 
attributes today, but sure, we can conservatively start with only allowing 
declaration attributes after the r-paren.


https://reviews.llvm.org/D41553



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

[PATCH] D42464: add prefix with '_' support for property name. Corresponding apple dev doc: https://developer.apple.com/library/content/qa/qa1908/_index.html

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton accepted this revision.
benhamilton added inline comments.
This revision is now accepted and ready to land.



Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:123
+  size_t Start = PropertyName.find_first_of('_');
+  assert(Start != llvm::StringRef::npos);
+  auto Prefix = PropertyName.substr(0, Start);

Can you also check that Start + 1 < PropertyName.length() (e.g., the string 
doesn't end with a _)?

I know the regular expression shouldn't match this, so an assert is probably 
good. (There is logic later which tries to access Start + 1 without checking.)



Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:129
+  auto RegexExp =
+  llvm::Regex(llvm::StringRef(validPropertyNameRegex(Acronyms, false)));
+  return RegexExp.match(llvm::StringRef(PropertyName.substr(Start + 1)));

Don't need to explicitly wrap in llvm::StringRef, that class has an implicit 
constructor from std::string.

https://github.com/llvm-mirror/llvm/blob/master/include/llvm/ADT/StringRef.h#L95




Comment at: test/clang-tidy/objc-property-declaration.m:15-31
+@interface Foo (Bar)
+@property(assign, nonatomic) int abc_NotCamelCase;
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'abc_NotCamelCase' 
not using lowerCamelCase style or not prefixed in a category, according to the 
Apple Coding Guidelines [objc-property-declaration]
+@property(assign, nonatomic) int abCD_camelCase;
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'abCD_camelCase' 
not using lowerCamelCase style or not prefixed in a category, according to the 
Apple Coding Guidelines [objc-property-declaration]
+// CHECK-FIXES: @property(assign, nonatomic) int abcd_camelCase;
+@property(assign, nonatomic) int abCD_NotCamelCase;

Can we add checks for properties whose names end with _?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42464



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

Re: [clang-tools-extra] r323658 - [clangd] Add a fallback directory for collected symbols with relative paths.

[Galina Kistanova via cfe-commits]

Hello Eric,

It look like some of your recent commits broke the test on one of our
builders:
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast
. . .
Failing Tests (1):
Extra Tools Unit Tests ::
clangd/./ClangdTests.exe/SymbolCollectorTest.SymbolRelativeWithFallback

The builder was already red and did not send notifications on the changes.
Please have a look?

Thanks

Galina

On Mon, Jan 29, 2018 at 7:13 AM, Eric Liu via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: ioeric
> Date: Mon Jan 29 07:13:29 2018
> New Revision: 323658
>
> URL: http://llvm.org/viewvc/llvm-project?rev=323658&view=rev
> Log:
> [clangd] Add a fallback directory for collected symbols with relative
> paths.
>
> Reviewers: hokein, sammccall
>
> Subscribers: klimek, ilya-biryukov, jkorous-apple, cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D42638
>
> Modified:
> clang-tools-extra/trunk/clangd/global-symbol-builder/
> GlobalSymbolBuilderMain.cpp
> clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
> clang-tools-extra/trunk/clangd/index/SymbolCollector.h
> clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
>
> Modified: clang-tools-extra/trunk/clangd/global-symbol-builder/
> GlobalSymbolBuilderMain.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/
> trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp?
> rev=323658&r1=323657&r2=323658&view=diff
> 
> ==
> --- 
> clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
> (original)
> +++ 
> clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
> Mon Jan 29 07:13:29 2018
> @@ -35,6 +35,14 @@ using clang::clangd::SymbolSlab;
>  namespace clang {
>  namespace clangd {
>
> +static llvm::cl::opt AssumedHeaderDir(
> +"assume-header-dir",
> +llvm::cl::desc("The index includes header that a symbol is defined
> in. "
> +   "If the absolute path cannot be determined (e.g. an "
> +   "in-memory VFS) then the relative path is resolved
> against "
> +   "this directory, which must be absolute. If this flag
> is "
> +   "not given, such headers will have relative paths."));
> +
>  class SymbolIndexActionFactory : public tooling::FrontendActionFactory {
>  public:
>SymbolIndexActionFactory(tooling::ExecutionContext *Ctx) : Ctx(Ctx) {}
> @@ -72,9 +80,11 @@ public:
>  IndexOpts.SystemSymbolFilter =
>  index::IndexingOptions::SystemSymbolFilterKind::All;
>  IndexOpts.IndexFunctionLocals = false;
> +auto CollectorOpts = SymbolCollector::Options();
> +CollectorOpts.FallbackDir = AssumedHeaderDir;
>  return new WrappedIndexAction(
> -std::make_shared(SymbolCollector::Options()),
> -IndexOpts, Ctx);
> +std::make_shared(std::move(CollectorOpts)),
> IndexOpts,
> +Ctx);
>}
>
>tooling::ExecutionContext *Ctx;
> @@ -98,6 +108,12 @@ int main(int argc, const char **argv) {
>  return 1;
>}
>
> +  if (!clang::clangd::AssumedHeaderDir.empty() &&
> +  !llvm::sys::path::is_absolute(clang::clangd::AssumedHeaderDir)) {
> +llvm::errs() << "--assume-header-dir must be an absolute path.\n";
> +return 1;
> +  }
> +
>auto Err = Executor->get()->execute(
>llvm::make_unique(
>Executor->get()->getExecutionContext()));
>
> Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/
> trunk/clangd/index/SymbolCollector.cpp?rev=323658&r1=323657&r2=323658&
> view=diff
> 
> ==
> --- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original)
> +++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Mon Jan 29
> 07:13:29 2018
> @@ -14,6 +14,7 @@
>  #include "clang/Basic/SourceManager.h"
>  #include "clang/Index/IndexSymbol.h"
>  #include "clang/Index/USRGeneration.h"
> +#include "llvm/Support/FileSystem.h"
>  #include "llvm/Support/MemoryBuffer.h"
>  #include "llvm/Support/Path.h"
>
> @@ -22,36 +23,42 @@ namespace clangd {
>
>  namespace {
>  // Make the Path absolute using the current working directory of the given
> -// SourceManager if the Path is not an absolute path.
> +// SourceManager if the Path is not an absolute path. If failed, this
> combine
> +// relative paths with \p FallbackDir to get an absolute path.
>  //
>  // The Path can be a path relative to the build directory, or retrieved
> from
>  // the SourceManager.
> -std::string makeAbsolutePath(const SourceManager &SM, StringRef Path) {
> +std::string makeAbsolutePath(const SourceManager &SM, StringRef Path,
> + StringRef FallbackDir) {
>llvm::SmallString<128> AbsolutePath(Path);
>if (std::error_code EC =
>SM.getFileManager().getVirtu

[PATCH] D42660: [PR32482] Fix bitfield layout for -mms-bitfield and pragma pack

[Alex Lorenz via Phabricator via cfe-commits]

arphaman created this revision.
arphaman added reviewers: ahatanak, rjmccall.
Herald added a subscriber: jkorous-apple.

Clang currently generates wrong record layout for `-mms-bitfield` and `#pragma 
pack`. https://godbolt.org/g/nQ4rVW shows how MSVC and GCC generate different 
layout to Clang.

This patch fixes the issue by ensuring that bitfields are not packed, but the 
record's alignment is still updated. This seems to be in line with logic used 
by GCC and MSVC.

rdar://36343145


Repository:
  rC Clang

https://reviews.llvm.org/D42660

Files:
  lib/AST/RecordLayoutBuilder.cpp
  test/CodeGen/mms-bitfields.c
  test/Sema/mms-bitfields.c

Index: test/Sema/mms-bitfields.c
===
--- test/Sema/mms-bitfields.c
+++ test/Sema/mms-bitfields.c
@@ -11,3 +11,18 @@
 
 // MS pads out bitfields between different types.
 static int arr[(sizeof(t) == 8) ? 1 : -1];
+
+#pragma pack (push,1)
+
+typedef unsigned int UINT32;
+
+struct Inner {
+  UINT32A:  1;
+  UINT32B:  1;
+  UINT32C:  1;
+  UINT32D: 30;
+} Inner;
+
+#pragma pack (pop)
+
+static int arr2[(sizeof(Inner) == 8) ? 1 : -1];
Index: test/CodeGen/mms-bitfields.c
===
--- test/CodeGen/mms-bitfields.c
+++ test/CodeGen/mms-bitfields.c
@@ -20,3 +20,46 @@
 } s3;
 
 // CHECK: %struct.s3 = type { i32, [4 x i8], %struct.s1 }
+
+// PR32482:
+
+#pragma pack (push,1)
+
+typedef unsigned int UINT32;
+
+struct Inner {
+  UINT32A:  1;
+  UINT32B:  1;
+  UINT32C:  1;
+  UINT32D: 30;
+} Inner;
+
+#pragma pack (pop)
+
+// CHECK: %struct.Inner = type { i32, i32 }
+
+// CHECK: %struct.A = type { i32, i32, i32 }
+
+#pragma pack(push, 1)
+
+union HEADER {
+  struct A {
+int :  3;  // Bits 2:0
+int a   :  9;  // Bits 11:3
+int :  12;  // Bits 23:12
+int b   :  17;  // Bits 40:24
+int :  7;  // Bits 47:41
+int c   :  4;  // Bits 51:48
+int :  4;  // Bits 55:52
+int d   :  3;  // Bits 58:56
+int :  5;  // Bits 63:59
+  } Bits;
+} HEADER;
+
+#pragma pack(pop)
+
+struct Inner variable = { 1,0,1, 21 };
+union HEADER hdr = {{1,2,3,4}};
+
+// CHECK: @variable = global { i8, [3 x i8], i8, i8, i8, i8 } { i8 5, [3 x i8] undef, i8 21, i8 0, i8 0, i8 0 }, align 1
+// CHECK: @hdr = global { { i8, i8, [2 x i8], i8, i8, i8, i8, i8, [3 x i8] } } { { i8, i8, [2 x i8], i8, i8, i8, i8, i8, [3 x i8] } { i8 8, i8 0, [2 x i8] undef, i8 2, i8 0, i8 0, i8 3, i8 4, [3 x i8] undef } }, align 1
Index: lib/AST/RecordLayoutBuilder.cpp
===
--- lib/AST/RecordLayoutBuilder.cpp
+++ lib/AST/RecordLayoutBuilder.cpp
@@ -1561,12 +1561,21 @@
   // But, if there's a #pragma pack in play, that takes precedent over
   // even the 'aligned' attribute, for non-zero-width bitfields.
   unsigned MaxFieldAlignmentInBits = Context.toBits(MaxFieldAlignment);
+  unsigned RecordFieldAlign = FieldAlign;
   if (!MaxFieldAlignment.isZero() && FieldSize) {
-UnpackedFieldAlign = std::min(UnpackedFieldAlign, MaxFieldAlignmentInBits);
-if (FieldPacked)
-  FieldAlign = UnpackedFieldAlign;
-else
-  FieldAlign = std::min(FieldAlign, MaxFieldAlignmentInBits);
+// #pragma pack does not affect bitfield layout in ms_struct, but the
+// record's alignment is still changed.
+if (IsMsStruct) {
+  RecordFieldAlign = std::min(FieldAlign, MaxFieldAlignmentInBits);
+} else {
+  UnpackedFieldAlign =
+  std::min(UnpackedFieldAlign, MaxFieldAlignmentInBits);
+  if (FieldPacked)
+FieldAlign = UnpackedFieldAlign;
+  else
+FieldAlign = std::min(FieldAlign, MaxFieldAlignmentInBits);
+  RecordFieldAlign = FieldAlign;
+}
   }
 
   // But, ms_struct just ignores all of that in unions, even explicit
@@ -1699,7 +1708,7 @@
   setSize(std::max(getSizeInBits(), getDataSizeInBits()));
 
   // Remember max struct/class alignment.
-  UpdateAlignment(Context.toCharUnitsFromBits(FieldAlign), 
+  UpdateAlignment(Context.toCharUnitsFromBits(RecordFieldAlign),
   Context.toCharUnitsFromBits(UnpackedFieldAlign));
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41885: [libcxxabi][demangler] Improve handling of variadic templates

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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

LGTM once you clean up the `#if 0`.




Comment at: src/cxa_demangle.cpp:260-261
+
+#if 0
+  void dump() const {
+char *Buffer = static_cast(std::malloc(1024));

erik.pilkington wrote:
> dexonsmith wrote:
> > Why is this behind `#if 0`?  Should you just use something like 
> > `LLVM_DUMP_METHOD`?
> Do you mean conditionally defined based on NDEBUG and marked 
> __attribute__((noinline,used)) for debuggers? I was just using it as a quick 
> & simple little helper for debugging.
Yup, those two things sound great to me.  You might need some platform-specific 
logic to define the attributes correctly.

I figured it was a quick and simple helper, like the `dump()` methods 
throughout LLVM.  Here's why I think you should change it:
- I imagine this will be useful in the future, too, and being able to call 
`dump()` from the debugger without recompiling is a great feature.
- `#if 0`s render as comments in some editors, and then bitrot because no one 
compiles them.


https://reviews.llvm.org/D41885



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

[PATCH] D42464: add prefix with '_' support for property name. Corresponding apple dev doc: https://developer.apple.com/library/content/qa/qa1908/_index.html

[Yan Zhang via Phabricator via cfe-commits]

Wizard updated this revision to Diff 131877.
Wizard marked 2 inline comments as done.
Wizard added a comment.

add more tests


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42464

Files:
  clang-tidy/objc/PropertyDeclarationCheck.cpp
  docs/clang-tidy/checks/objc-property-declaration.rst
  test/clang-tidy/objc-property-declaration-custom.m
  test/clang-tidy/objc-property-declaration.m

Index: test/clang-tidy/objc-property-declaration.m
===
--- test/clang-tidy/objc-property-declaration.m
+++ test/clang-tidy/objc-property-declaration.m
@@ -3,11 +3,31 @@
 
 @interface Foo
 @property(assign, nonatomic) int NotCamelCase;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'NotCamelCase' should use lowerCamelCase style, according to the Apple Coding Guidelines [objc-property-declaration]
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'NotCamelCase' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
 // CHECK-FIXES: @property(assign, nonatomic) int notCamelCase;
 @property(assign, nonatomic) int camelCase;
 @property(strong, nonatomic) NSString *URLString;
 @property(strong, nonatomic) NSString *bundleID;
 @property(strong, nonatomic) NSString *URL_string;
-// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'URL_string' should use lowerCamelCase style, according to the Apple Coding Guidelines [objc-property-declaration]
+// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'URL_string' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
 @end
+
+@interface Foo (Bar)
+@property(assign, nonatomic) int abc_NotCamelCase;
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'abc_NotCamelCase' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
+@property(assign, nonatomic) int abCD_camelCase;
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'abCD_camelCase' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
+// CHECK-FIXES: @property(assign, nonatomic) int abcd_camelCase;
+@property(assign, nonatomic) int abCD_NotCamelCase;
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'abCD_NotCamelCase' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
+// CHECK-FIXES: @property(assign, nonatomic) int abcd_notCamelCase;
+@property(assign, nonatomic) int wrongFormat_;
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'wrongFormat_' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
+@property(strong, nonatomic) NSString *URLStr;
+@property(assign, nonatomic) int abc_camelCase;
+@end
+
+@interface Foo ()
+@property(assign, nonatomic) int abc_inClassExtension;
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'abc_inClassExtension' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
+@end
\ No newline at end of file
Index: test/clang-tidy/objc-property-declaration-custom.m
===
--- test/clang-tidy/objc-property-declaration-custom.m
+++ test/clang-tidy/objc-property-declaration-custom.m
@@ -6,9 +6,9 @@
 
 @interface Foo
 @property(assign, nonatomic) int AbcNotRealPrefix;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'AbcNotRealPrefix' should use lowerCamelCase style, according to the Apple Coding Guidelines [objc-property-declaration]
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'AbcNotRealPrefix' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
 // CHECK-FIXES: @property(assign, nonatomic) int abcNotRealPrefix;
 @property(assign, nonatomic) int ABCCustomPrefix;
 @property(strong, nonatomic) NSString *ABC_custom_prefix;
-// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'ABC_custom_prefix' should use lowerCamelCase style, according to the Apple Coding Guidelines [objc-property-declaration]
+// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'ABC_custom_prefix' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
 @end
Index: docs/clang-tidy/checks/objc-property-declaration.rst
===
--- docs/clang-tidy/checks/objc-property-declaration.rst
+++ docs/clang-tidy/checks/objc-property-declaration.rst
@@ -32,6 +32,15 @@
 
 The corresponding style rule: https://developer.apple.com/library/content/documenta

[clang-tools-extra] r323703 - [clangd] Fix r323658 test failure on windows.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Mon Jan 29 14:28:08 2018
New Revision: 323703

URL: http://llvm.org/viewvc/llvm-project?rev=323703&view=rev
Log:
[clangd] Fix r323658 test failure on windows.

Modified:
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
clang-tools-extra/trunk/unittests/clangd/TestFS.cpp
clang-tools-extra/trunk/unittests/clangd/TestFS.h

Modified: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp?rev=323703&r1=323702&r2=323703&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp Mon Jan 
29 14:28:08 2018
@@ -7,6 +7,7 @@
 //
 
//===--===//
 
+#include "TestFS.h"
 #include "index/SymbolCollector.h"
 #include "index/SymbolYAML.h"
 #include "clang/Basic/FileManager.h"
@@ -44,7 +45,7 @@ MATCHER_P(Snippet, S, "") {
   return arg.CompletionSnippetInsertText == S;
 }
 MATCHER_P(QName, Name, "") { return (arg.Scope + arg.Name).str() == Name; }
-MATCHER_P(Path, P, "") { return arg.CanonicalDeclaration.FilePath == P; }
+MATCHER_P(CPath, P, "") { return arg.CanonicalDeclaration.FilePath == P; }
 
 namespace clang {
 namespace clangd {
@@ -156,15 +157,16 @@ TEST_F(SymbolCollectorTest, SymbolRelati
   CollectorOpts.IndexMainFiles = false;
   runSymbolCollector("class Foo {};", /*Main=*/"");
   EXPECT_THAT(Symbols,
-  UnorderedElementsAre(AllOf(QName("Foo"), Path("symbols.h";
+  UnorderedElementsAre(AllOf(QName("Foo"), CPath("symbols.h";
 }
 
 TEST_F(SymbolCollectorTest, SymbolRelativeWithFallback) {
   CollectorOpts.IndexMainFiles = false;
-  CollectorOpts.FallbackDir = "/cwd";
+  CollectorOpts.FallbackDir = getVirtualTestRoot();
   runSymbolCollector("class Foo {};", /*Main=*/"");
-  EXPECT_THAT(Symbols, UnorderedElementsAre(
-   AllOf(QName("Foo"), Path("/cwd/symbols.h";
+  EXPECT_THAT(Symbols,
+  UnorderedElementsAre(AllOf(
+  QName("Foo"), CPath(getVirtualTestFilePath("symbols.h");
 }
 
 TEST_F(SymbolCollectorTest, IncludeEnums) {

Modified: clang-tools-extra/trunk/unittests/clangd/TestFS.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/TestFS.cpp?rev=323703&r1=323702&r2=323703&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/TestFS.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/TestFS.cpp Mon Jan 29 14:28:08 2018
@@ -159,7 +159,7 @@ MockCompilationDatabase::getCompileComma
   std::move(CommandLine), "")};
 }
 
-static const char *getVirtualTestRoot() {
+const char *getVirtualTestRoot() {
 #ifdef LLVM_ON_WIN32
   return "C:\\clangd-test";
 #else

Modified: clang-tools-extra/trunk/unittests/clangd/TestFS.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/TestFS.h?rev=323703&r1=323702&r2=323703&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/TestFS.h (original)
+++ clang-tools-extra/trunk/unittests/clangd/TestFS.h Mon Jan 29 14:28:08 2018
@@ -45,6 +45,9 @@ public:
   std::vector ExtraClangFlags;
 };
 
+// Returns an absolute (fake) test directory for this OS.
+const char *getVirtualTestRoot();
+
 // Returns a suitable absolute path for this OS.
 llvm::SmallString<32> getVirtualTestFilePath(PathRef File);
 


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

[PATCH] D42464: add prefix with '_' support for property name. Corresponding apple dev doc: https://developer.apple.com/library/content/qa/qa1908/_index.html

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton accepted this revision.
benhamilton added a comment.

Thanks!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42464



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

Re: [clang-tools-extra] r323658 - [clangd] Add a fallback directory for collected symbols with relative paths.

[Eric Liu via cfe-commits]

Thanks Galina! r323703 should fix this.

On Mon, Jan 29, 2018 at 11:09 PM Galina Kistanova 
wrote:

> Hello Eric,
>
> It look like some of your recent commits broke the test on one of our
> builders:
>
> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast
> . . .
> Failing Tests (1):
> Extra Tools Unit Tests ::
> clangd/./ClangdTests.exe/SymbolCollectorTest.SymbolRelativeWithFallback
>
> The builder was already red and did not send notifications on the changes.
> Please have a look?
>
> Thanks
>
>
> Galina
>
> On Mon, Jan 29, 2018 at 7:13 AM, Eric Liu via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: ioeric
>> Date: Mon Jan 29 07:13:29 2018
>> New Revision: 323658
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=323658&view=rev
>> Log:
>> [clangd] Add a fallback directory for collected symbols with relative
>> paths.
>>
>> Reviewers: hokein, sammccall
>>
>> Subscribers: klimek, ilya-biryukov, jkorous-apple, cfe-commits
>>
>> Differential Revision: https://reviews.llvm.org/D42638
>>
>> Modified:
>>
>> clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
>> clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
>> clang-tools-extra/trunk/clangd/index/SymbolCollector.h
>> clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
>>
>> Modified:
>> clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp?rev=323658&r1=323657&r2=323658&view=diff
>>
>> ==
>> ---
>> clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
>> (original)
>> +++
>> clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
>> Mon Jan 29 07:13:29 2018
>> @@ -35,6 +35,14 @@ using clang::clangd::SymbolSlab;
>>  namespace clang {
>>  namespace clangd {
>>
>> +static llvm::cl::opt AssumedHeaderDir(
>> +"assume-header-dir",
>> +llvm::cl::desc("The index includes header that a symbol is defined
>> in. "
>> +   "If the absolute path cannot be determined (e.g. an "
>> +   "in-memory VFS) then the relative path is resolved
>> against "
>> +   "this directory, which must be absolute. If this flag
>> is "
>> +   "not given, such headers will have relative paths."));
>> +
>>  class SymbolIndexActionFactory : public tooling::FrontendActionFactory {
>>  public:
>>SymbolIndexActionFactory(tooling::ExecutionContext *Ctx) : Ctx(Ctx) {}
>> @@ -72,9 +80,11 @@ public:
>>  IndexOpts.SystemSymbolFilter =
>>  index::IndexingOptions::SystemSymbolFilterKind::All;
>>  IndexOpts.IndexFunctionLocals = false;
>> +auto CollectorOpts = SymbolCollector::Options();
>> +CollectorOpts.FallbackDir = AssumedHeaderDir;
>>  return new WrappedIndexAction(
>> -std::make_shared(SymbolCollector::Options()),
>> -IndexOpts, Ctx);
>> +std::make_shared(std::move(CollectorOpts)),
>> IndexOpts,
>> +Ctx);
>>}
>>
>>tooling::ExecutionContext *Ctx;
>> @@ -98,6 +108,12 @@ int main(int argc, const char **argv) {
>>  return 1;
>>}
>>
>> +  if (!clang::clangd::AssumedHeaderDir.empty() &&
>> +  !llvm::sys::path::is_absolute(clang::clangd::AssumedHeaderDir)) {
>> +llvm::errs() << "--assume-header-dir must be an absolute path.\n";
>> +return 1;
>> +  }
>> +
>>auto Err = Executor->get()->execute(
>>llvm::make_unique(
>>Executor->get()->getExecutionContext()));
>>
>> Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=323658&r1=323657&r2=323658&view=diff
>>
>> ==
>> --- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original)
>> +++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Mon Jan 29
>> 07:13:29 2018
>> @@ -14,6 +14,7 @@
>>  #include "clang/Basic/SourceManager.h"
>>  #include "clang/Index/IndexSymbol.h"
>>  #include "clang/Index/USRGeneration.h"
>> +#include "llvm/Support/FileSystem.h"
>>  #include "llvm/Support/MemoryBuffer.h"
>>  #include "llvm/Support/Path.h"
>>
>> @@ -22,36 +23,42 @@ namespace clangd {
>>
>>  namespace {
>>  // Make the Path absolute using the current working directory of the
>> given
>> -// SourceManager if the Path is not an absolute path.
>> +// SourceManager if the Path is not an absolute path. If failed, this
>> combine
>> +// relative paths with \p FallbackDir to get an absolute path.
>>  //
>>  // The Path can be a path relative to the build directory, or retrieved
>> from
>>  // the SourceManager.
>> -std::string makeAbsolutePath(const SourceManager &SM, StringRef Path) {
>> +std::string 

[PATCH] D42578: [AMDGPU] Add ds_fadd, ds_fmin, ds_fmax builtins functions

[Brian Sumner via Phabricator via cfe-commits]

b-sumner added a comment.

Should we expect that the last 3 arguments have any effect?  Do we want to test 
to ensure they have the expected effects?


Repository:
  rC Clang

https://reviews.llvm.org/D42578



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

[PATCH] D41318: Start setting dso_local in clang

[Rafael Avila de Espindola via Phabricator via cfe-commits]

espindola updated this revision to Diff 131878.
espindola added a comment.

Rebased. Ping


https://reviews.llvm.org/D41318

Files:
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGVTT.cpp
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGen/dso-local-executable.c
  clang/test/CodeGen/mbackchain-2.c
  clang/test/CodeGen/mbackchain-3.c
  clang/test/CodeGen/mips-vector-return.c
  clang/test/CodeGen/split-stacks.c
  clang/test/CodeGenCXX/debug-info-static-member.cpp
  clang/test/CodeGenCXX/debug-info-template.cpp
  clang/test/CodeGenCXX/dso-local-executable.cpp
  clang/test/CodeGenCXX/float16-declarations.cpp
  clang/test/CodeGenCXX/split-stacks.cpp
  clang/test/Driver/lanai-unknown-unknown.cpp
  clang/test/Driver/le32-unknown-nacl.cpp
  clang/test/Driver/le64-unknown-unknown.cpp
  clang/test/Driver/riscv32-toolchain.c
  clang/test/Driver/riscv64-toolchain.c
  clang/test/Frontend/ast-codegen.c

Index: clang/test/Frontend/ast-codegen.c
===
--- clang/test/Frontend/ast-codegen.c
+++ clang/test/Frontend/ast-codegen.c
@@ -5,9 +5,9 @@
 // CHECK: module asm "foo"
 __asm__("foo");
 
-// CHECK: @g0 = common global i32 0, align 4
+// CHECK: @g0 = common dso_local global i32 0, align 4
 int g0;
 
-// CHECK: define i32 @f0()
+// CHECK: define dso_local i32 @f0()
 int f0() {
 }
Index: clang/test/Driver/riscv64-toolchain.c
===
--- clang/test/Driver/riscv64-toolchain.c
+++ clang/test/Driver/riscv64-toolchain.c
@@ -10,82 +10,82 @@
 typedef __PTRDIFF_TYPE__ ptrdiff_t;
 typedef __WCHAR_TYPE__ wchar_t;
 
-// CHECK: @align_c = global i32 1
+// CHECK: @align_c = dso_local global i32 1
 int align_c = __alignof(char);
 
-// CHECK: @align_s = global i32 2
+// CHECK: @align_s = dso_local global i32 2
 int align_s = __alignof(short);
 
-// CHECK: @align_i = global i32 4
+// CHECK: @align_i = dso_local global i32 4
 int align_i = __alignof(int);
 
-// CHECK: @align_wc = global i32 4
+// CHECK: @align_wc = dso_local global i32 4
 int align_wc = __alignof(wchar_t);
 
-// CHECK: @align_l = global i32 8
+// CHECK: @align_l = dso_local global i32 8
 int align_l = __alignof(long);
 
-// CHECK: @align_ll = global i32 8
+// CHECK: @align_ll = dso_local global i32 8
 int align_ll = __alignof(long long);
 
-// CHECK: @align_p = global i32 8
+// CHECK: @align_p = dso_local global i32 8
 int align_p = __alignof(void*);
 
-// CHECK: @align_f = global i32 4
+// CHECK: @align_f = dso_local global i32 4
 int align_f = __alignof(float);
 
-// CHECK: @align_d = global i32 8
+// CHECK: @align_d = dso_local global i32 8
 int align_d = __alignof(double);
 
-// CHECK: @align_ld = global i32 16
+// CHECK: @align_ld = dso_local global i32 16
 int align_ld = __alignof(long double);
 
-// CHECK: @align_vl = global i32 8
+// CHECK: @align_vl = dso_local global i32 8
 int align_vl = __alignof(va_list);
 
 // Check types
 
-// CHECK: define zeroext i8 @check_char()
+// CHECK: define dso_local zeroext i8 @check_char()
 char check_char() { return 0; }
 
-// CHECK: define signext i16 @check_short()
+// CHECK: define dso_local signext i16 @check_short()
 short check_short() { return 0; }
 
-// CHECK: define signext i32 @check_int()
+// CHECK: define dso_local signext i32 @check_int()
 int check_int() { return 0; }
 
-// CHECK: define signext i32 @check_wchar_t()
+// CHECK: define dso_local signext i32 @check_wchar_t()
 int check_wchar_t() { return 0; }
 
-// CHECK: define i64 @check_long()
+// CHECK: define dso_local i64 @check_long()
 long check_long() { return 0; }
 
-// CHECK: define i64 @check_longlong()
+// CHECK: define dso_local i64 @check_longlong()
 long long check_longlong() { return 0; }
 
-// CHECK: define zeroext i8 @check_uchar()
+// CHECK: define dso_local zeroext i8 @check_uchar()
 unsigned char check_uchar() { return 0; }
 
-// CHECK: define zeroext i16 @check_ushort()
+// CHECK: define dso_local zeroext i16 @check_ushort()
 unsigned short check_ushort() { return 0; }
 
-// CHECK: define signext i32 @check_uint()
+// CHECK: define dso_local signext i32 @check_uint()
 unsigned int check_uint() { return 0; }
 
-// CHECK: define i64 @check_ulong()
+// CHECK: define dso_local i64 @check_ulong()
 unsigned long check_ulong() { return 0; }
 
-// CHECK: define i64 @check_ulonglong()
+// CHECK: define dso_local i64 @check_ulonglong()
 unsigned long long check_ulonglong() { return 0; }
 
-// CHECK: define i64 @check_size_t()
+// CHECK: define dso_local i64 @check_size_t()
 size_t check_size_t() { return 0; }
 
-// CHECK: define float @check_float()
+// CHECK: define dso_local float @check_float()
 float check_float() { return 0; }
 
-// CHECK: define double @check_double()
+// CHECK: define dso_local double @check_double()
 double check_double() { return 0; }
 
-// CHECK: define fp128 @check_longdouble()
+// CHECK: defi