[PATCH] D46942: Add vfs::FileSystem::getRealPath

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: lib/Basic/FileManager.cpp:537
 
-#ifdef LLVM_ON_UNIX
-  char CanonicalNameBuf[PATH_MAX];
-  if (realpath(Dir->getName().str().c_str(), CanonicalNameBuf))
+  SmallString CanonicalNameBuf;
+  if (!FS->getRealPath(Dir->getName(), CanonicalNameBuf))

bkramer wrote:
> PATH_MAX is not a standard thing and probably not there on windows. I'd just 
> hardcode it to 256.
PATH_MAX on unit is 4096. Let's go with 4096.


Repository:
  rC Clang

https://reviews.llvm.org/D46942



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

[PATCH] D46942: Add vfs::FileSystem::getRealPath

[Eric Liu via Phabricator via cfe-commits]

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

- s/PATH_MAX/4096/


Repository:
  rC Clang

https://reviews.llvm.org/D46942

Files:
  include/clang/Basic/VirtualFileSystem.h
  lib/Basic/FileManager.cpp
  lib/Basic/VirtualFileSystem.cpp


Index: lib/Basic/VirtualFileSystem.cpp
===
--- lib/Basic/VirtualFileSystem.cpp
+++ lib/Basic/VirtualFileSystem.cpp
@@ -139,6 +139,11 @@
   return llvm::sys::fs::make_absolute(WorkingDir.get(), Path);
 }
 
+std::error_code FileSystem::getRealPath(const Twine ,
+SmallVectorImpl ) const {
+  return errc::operation_not_permitted;
+}
+
 bool FileSystem::exists(const Twine ) {
   auto Status = status(Path);
   return Status && Status->exists();
@@ -236,6 +241,8 @@
 
   llvm::ErrorOr getCurrentWorkingDirectory() const override;
   std::error_code setCurrentWorkingDirectory(const Twine ) override;
+  std::error_code getRealPath(const Twine ,
+  SmallVectorImpl ) const override;
 };
 
 } // namespace
@@ -274,6 +281,12 @@
   return llvm::sys::fs::set_current_path(Path);
 }
 
+std::error_code
+RealFileSystem::getRealPath(const Twine ,
+SmallVectorImpl ) const {
+  return llvm::sys::fs::real_path(Path, Output);
+}
+
 IntrusiveRefCntPtr vfs::getRealFileSystem() {
   static IntrusiveRefCntPtr FS = new RealFileSystem();
   return FS;
Index: lib/Basic/FileManager.cpp
===
--- lib/Basic/FileManager.cpp
+++ lib/Basic/FileManager.cpp
@@ -534,23 +534,9 @@
 
   StringRef CanonicalName(Dir->getName());
 
-#ifdef LLVM_ON_UNIX
-  char CanonicalNameBuf[PATH_MAX];
-  if (realpath(Dir->getName().str().c_str(), CanonicalNameBuf))
+  SmallString<4096> CanonicalNameBuf;
+  if (!FS->getRealPath(Dir->getName(), CanonicalNameBuf))
 CanonicalName = StringRef(CanonicalNameBuf).copy(CanonicalNameStorage);
-#else
-  SmallString<256> CanonicalNameBuf(CanonicalName);
-  llvm::sys::fs::make_absolute(CanonicalNameBuf);
-  llvm::sys::path::native(CanonicalNameBuf);
-  // We've run into needing to remove '..' here in the wild though, so
-  // remove it.
-  // On Windows, symlinks are significantly less prevalent, so removing
-  // '..' is pretty safe.
-  // Ideally we'd have an equivalent of `realpath` and could implement
-  // sys::fs::canonical across all the platforms.
-  llvm::sys::path::remove_dots(CanonicalNameBuf, /* remove_dot_dot */ true);
-  CanonicalName = StringRef(CanonicalNameBuf).copy(CanonicalNameStorage);
-#endif
 
   CanonicalDirNames.insert(std::make_pair(Dir, CanonicalName));
   return CanonicalName;
Index: include/clang/Basic/VirtualFileSystem.h
===
--- include/clang/Basic/VirtualFileSystem.h
+++ include/clang/Basic/VirtualFileSystem.h
@@ -248,6 +248,12 @@
   /// Get the working directory of this file system.
   virtual llvm::ErrorOr getCurrentWorkingDirectory() const = 0;
 
+  /// Gets real path of \p Path e.g. collapse all . and .. patterns, resolve
+  /// symlinks. For real file system, this uses `llvm::sys::fs::real_path`.
+  /// This returns errc::operation_not_permitted if not implemented by 
subclass.
+  virtual std::error_code getRealPath(const Twine ,
+  SmallVectorImpl ) const;
+
   /// Check whether a file exists. Provided for convenience.
   bool exists(const Twine );
 


Index: lib/Basic/VirtualFileSystem.cpp
===
--- lib/Basic/VirtualFileSystem.cpp
+++ lib/Basic/VirtualFileSystem.cpp
@@ -139,6 +139,11 @@
   return llvm::sys::fs::make_absolute(WorkingDir.get(), Path);
 }
 
+std::error_code FileSystem::getRealPath(const Twine ,
+SmallVectorImpl ) const {
+  return errc::operation_not_permitted;
+}
+
 bool FileSystem::exists(const Twine ) {
   auto Status = status(Path);
   return Status && Status->exists();
@@ -236,6 +241,8 @@
 
   llvm::ErrorOr getCurrentWorkingDirectory() const override;
   std::error_code setCurrentWorkingDirectory(const Twine ) override;
+  std::error_code getRealPath(const Twine ,
+  SmallVectorImpl ) const override;
 };
 
 } // namespace
@@ -274,6 +281,12 @@
   return llvm::sys::fs::set_current_path(Path);
 }
 
+std::error_code
+RealFileSystem::getRealPath(const Twine ,
+SmallVectorImpl ) const {
+  return llvm::sys::fs::real_path(Path, Output);
+}
+
 IntrusiveRefCntPtr vfs::getRealFileSystem() {
   static IntrusiveRefCntPtr FS = new RealFileSystem();
   return FS;
Index: lib/Basic/FileManager.cpp
===
--- lib/Basic/FileManager.cpp
+++ lib/Basic/FileManager.cpp
@@ -534,23 +534,9 @@
 
   StringRef CanonicalName(Dir->getName());
 
-#ifdef LLVM_ON_UNIX
-  char 

[PATCH] D44954: [clangd] Add "member" symbols to the index

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

A few questions regarding class members. To pinpoint some interesting cases and 
agree on how we want those to behave in the long run.

How do we handle template specializations? What will the qualified names of 
those instantiations be?
I.e. how do I query for `push_back` inside a vector?  Which of the following 
queries should produce a result?

- `vector::push_back`. Should it match both `vector::push_back` and 
`vector::push_back` or only the first one?
- `vector::push_back`
- `vector::push_back`

What scopes will non-scoped enum members have?
E.g. if I have `enum En { A,B,C}`, which of the following queries will and 
won't find enumerators?

- `En::A`
- `::A`
- `A`




Comment at: clangd/CodeComplete.cpp:932
 Req.Query = Filter->pattern();
+Req.DeclContexts = {Decl::Kind::Namespace, Decl::Kind::TranslationUnit,
+Decl::Kind::LinkageSpec, Decl::Kind::Enum};

malaperle wrote:
> I want to add a comment here, but I want to make sure I understand why in the 
> first place we were not indexing symbols outside these contexts for the 
> purpose of code completion. Is it because those will be available by Sema 
> code completion anyway?
C++ lookup rules inside classes are way more complicated than in namespaces and 
we can't possibly hope to give a decent approximation for those.
Moreover, completion inside classes does not require any non-local information, 
so there does not seem to be a win when using the index anyway.
So we'd rather rely on clang to do completion there, it will give way more 
useful results than any index implementation.



Comment at: clangd/index/Index.h:160
+  /// The Decl::Kind for the context of the symbol, i.e. what contains it.
+  Decl::Kind DeclContextKind;
+  /// Whether or not this is an enumerator inside a scoped enum (C++11).

How do we use `DeclContextKind`?
Why did we decide to not go with a `bool ForCompletion` instead? (I'm probably 
missing a conversation in the workspaceSymbol review, could you point me to 
those instead?)

I'm asking because clang enums are very detailed and designed for use in the 
compiler, using them in the index seems to complicate things.
It feels we don't need this level of detail in the symbols. Similar to how we 
don't store the whole structural type, but rely on string representation of 
completion label instead.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44954



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

[PATCH] D46958: [ASTImporter] Fix missing implict CXXRecordDecl

[Gabor Marton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL332588: [ASTImporter] Fix missing implict CXXRecordDecl 
(authored by martong, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D46958

Files:
  cfe/trunk/lib/AST/ASTImporter.cpp
  cfe/trunk/unittests/AST/ASTImporterTest.cpp


Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -2110,7 +2110,7 @@
   D2 = D2CXX;
   D2->setAccess(D->getAccess());
   D2->setLexicalDeclContext(LexicalDC);
-  if (!DCXX->getDescribedClassTemplate())
+  if (!DCXX->getDescribedClassTemplate() || DCXX->isImplicit())
 LexicalDC->addDeclInternal(D2);
 
   Importer.Imported(D, D2);
Index: cfe/trunk/unittests/AST/ASTImporterTest.cpp
===
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp
@@ -1348,7 +1348,7 @@
   Verifier.match(To, cxxRecordDecl(hasFieldOrder({"a", "b", "c"};
 }
 
-TEST_P(ASTImporterTestBase, DISABLED_ShouldImportImplicitCXXRecordDecl) {
+TEST_P(ASTImporterTestBase, ShouldImportImplicitCXXRecordDecl) {
   Decl *From, *To;
   std::tie(From, To) = getImportedDecl(
   R"(


Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -2110,7 +2110,7 @@
   D2 = D2CXX;
   D2->setAccess(D->getAccess());
   D2->setLexicalDeclContext(LexicalDC);
-  if (!DCXX->getDescribedClassTemplate())
+  if (!DCXX->getDescribedClassTemplate() || DCXX->isImplicit())
 LexicalDC->addDeclInternal(D2);
 
   Importer.Imported(D, D2);
Index: cfe/trunk/unittests/AST/ASTImporterTest.cpp
===
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp
@@ -1348,7 +1348,7 @@
   Verifier.match(To, cxxRecordDecl(hasFieldOrder({"a", "b", "c"};
 }
 
-TEST_P(ASTImporterTestBase, DISABLED_ShouldImportImplicitCXXRecordDecl) {
+TEST_P(ASTImporterTestBase, ShouldImportImplicitCXXRecordDecl) {
   Decl *From, *To;
   std::tie(From, To) = getImportedDecl(
   R"(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46942: Add vfs::FileSystem::getRealPath

[Benjamin Kramer via Phabricator via cfe-commits]

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

Looks good. Please watch the windows buildbots carefully after landing this.




Comment at: lib/Basic/FileManager.cpp:537
 
-#ifdef LLVM_ON_UNIX
-  char CanonicalNameBuf[PATH_MAX];
-  if (realpath(Dir->getName().str().c_str(), CanonicalNameBuf))
+  SmallString CanonicalNameBuf;
+  if (!FS->getRealPath(Dir->getName(), CanonicalNameBuf))

PATH_MAX is not a standard thing and probably not there on windows. I'd just 
hardcode it to 256.


Repository:
  rC Clang

https://reviews.llvm.org/D46942



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

[PATCH] D46891: [StaticAnalyzer] Added a getLValue method to ProgramState for bases

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus added a comment.

Thanks for the review!

If you like the new overload, can you please commit this for me?


https://reviews.llvm.org/D46891



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

[PATCH] D43667: [clang-doc] Implement a YAML generator

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clang-doc/Representation.h:238
 
+  // Non-const accessors for YAML output.
+  std::vector () { return NamespaceInfos; }

There are two alternatives to avoid this:
1) Pull the info storage into a separate struct e.g. `struct InfoSet::Infos`, 
and add a constructor setter `setInfos(Infos &&)`;
2) `friend yaml::MappingTraits;`



Comment at: clang-doc/generators/CMakeLists.txt:3
+
+add_clang_library(clangDocGenerators
+  YAMLGenerator.cpp

Why is this a separate library?



Comment at: clang-doc/generators/Generators.h:24
+
+class Generator {
+public:

Please add documentation.



Comment at: clang-doc/generators/Generators.h:26
+public:
+  Generator(std::unique_ptr , llvm::StringRef Root,
+llvm::StringRef Format)

The parameters are not trivial. Could you add documentation?



Comment at: clang-doc/generators/Generators.h:26
+public:
+  Generator(std::unique_ptr , llvm::StringRef Root,
+llvm::StringRef Format)

ioeric wrote:
> The parameters are not trivial. Could you add documentation?
Passing a reference to a unique pointer seems a bit strange. If `Generator` 
doesn't own IS, it should be passed by reference; otherwise, this should be 
passed by value.



Comment at: clang-doc/generators/Generators.h:28
+llvm::StringRef Format)
+  : IS(IS), Root(Root), Format(Format) {}
+  virtual ~Generator() = default;

Have you considered passing a output stream instead of passing in a directory 
and writing output to a hardcoded name? And I think `Root` (or a output stream) 
should be passed in via `generate`instead the constructor. 



Comment at: clang-doc/generators/Generators.h:39
+
+class YAMLGenerator : public Generator {
+public:

Please add documentation.



Comment at: clang-doc/generators/Generators.h:49
+
+class GeneratorFactory {
+public:

Please add documentation and explain why this is needed. 



Comment at: clang-doc/generators/Generators.h:49
+
+class GeneratorFactory {
+public:

ioeric wrote:
> Please add documentation and explain why this is needed. 
If you plan to plugin in more generators (e.g. in your customized build of 
clang-doc), consider using `llvm::Registry` which allows you to link in new 
generators without having to modify code. See 
`clang::clangd::URISchemeRegistry` for sample usage.



Comment at: clang-doc/generators/YAMLGenerator.cpp:223
+
+template <> struct MappingTraits {
+  static void mapping(IO , std::unique_ptr ) {

YAML mapping for `unique_ptr` is a bit unusual. I wonder whether this would 
work all the time e.g. if the unique_ptr has not been allocated. 



Comment at: clang-doc/generators/YAMLGenerator.cpp:250
+  std::error_code OutErrorInfo;
+  llvm::raw_fd_ostream OS(Path, OutErrorInfo, llvm::sys::fs::F_None);
+  if (OutErrorInfo != OK) {

Note that this would only work on real file system. You would not be able to 
run this in unit tests, and some tool executors run on virtual file system as 
well, so if you do go with directory, you would also want to pass in a VFS 
(`clang/Basic/VirtualFileSystem.h`).


https://reviews.llvm.org/D43667



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

[PATCH] D47007: [Sanitizer] CStringChecker fix for strlcpy when no bytes are copied to the dest buffer

[David CARLIER via Phabricator via cfe-commits]

devnexen created this revision.
devnexen added reviewers: NoQ, george.karpenkov.
devnexen created this object with visibility "All Users".
Herald added a subscriber: cfe-commits.

Again strlc* does not return a pointer so the zero size case does not fit.


Repository:
  rC Clang

https://reviews.llvm.org/D47007

Files:
  lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  test/Analysis/bsd-string.c


Index: test/Analysis/bsd-string.c
===
--- test/Analysis/bsd-string.c
+++ test/Analysis/bsd-string.c
@@ -38,3 +38,8 @@
   size_t len = strlcat(buf, "defg", 4);
   clang_analyzer_eval(len == 7); // expected-warning{{TRUE}}
 }
+
+int f7() {
+  char buf[8];
+  return strlcpy(buf, "1234567", 0); // no-crash
+}
Index: lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -1652,7 +1652,11 @@
 
 // If the size is known to be zero, we're done.
 if (StateZeroSize && !StateNonZeroSize) {
-  StateZeroSize = StateZeroSize->BindExpr(CE, LCtx, DstVal);
+  if (returnPtr) {
+StateZeroSize = StateZeroSize->BindExpr(CE, LCtx, DstVal);
+  } else {
+StateZeroSize = StateZeroSize->BindExpr(CE, LCtx, *lenValNL);
+  }
   C.addTransition(StateZeroSize);
   return;
 }


Index: test/Analysis/bsd-string.c
===
--- test/Analysis/bsd-string.c
+++ test/Analysis/bsd-string.c
@@ -38,3 +38,8 @@
   size_t len = strlcat(buf, "defg", 4);
   clang_analyzer_eval(len == 7); // expected-warning{{TRUE}}
 }
+
+int f7() {
+  char buf[8];
+  return strlcpy(buf, "1234567", 0); // no-crash
+}
Index: lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -1652,7 +1652,11 @@
 
 // If the size is known to be zero, we're done.
 if (StateZeroSize && !StateNonZeroSize) {
-  StateZeroSize = StateZeroSize->BindExpr(CE, LCtx, DstVal);
+  if (returnPtr) {
+StateZeroSize = StateZeroSize->BindExpr(CE, LCtx, DstVal);
+  } else {
+StateZeroSize = StateZeroSize->BindExpr(CE, LCtx, *lenValNL);
+  }
   C.addTransition(StateZeroSize);
   return;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r332458 - [AST] Added a helper to extract a user-friendly text of a comment.

[Ilya Biryukov via cfe-commits]

Terribly sorry for the breakage and many thanks for fixing this!

On Thu, May 17, 2018 at 9:04 AM Clement Courbet  wrote:

> I should have fixed it in r332576.
>
> On Wed, May 16, 2018 at 11:49 PM, Galina Kistanova via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Also few other builders are affected:
>>
>> http://lab.llvm.org:8011/builders/clang-x86_64-linux-abi-test
>> http://lab.llvm.org:8011/builders/clang-lld-x86_64-2stage
>> http://lab.llvm.org:8011/builders/clang-with-lto-ubuntu
>>
>>
>> Thanks
>>
>> Galina
>>
>> On Wed, May 16, 2018 at 12:58 PM, Galina Kistanova 
>> wrote:
>>
>>> Hello Ilya,
>>>
>>> This commit broke build step for couple of our builders:
>>>
>>> http://lab.llvm.org:8011/builders/clang-with-lto-ubuntu/builds/8541
>>> http://lab.llvm.org:8011/builders/clang-with-thin-lto-ubuntu
>>>
>>> . . .
>>> FAILED:
>>> tools/clang/unittests/AST/CMakeFiles/ASTTests.dir/CommentTextTest.cpp.o
>>> /usr/bin/c++   -DGTEST_HAS_RTTI=0 -DGTEST_HAS_TR1_TUPLE=0
>>> -DGTEST_LANG_CXX11=1 -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS
>>> -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/clang/unittests/AST
>>> -I/home/buildslave/buildslave1a/clang-with-lto-ubuntu/llvm.src/tools/clang/unittests/AST
>>> -I/home/buildslave/buildslave1a/clang-with-lto-ubuntu/llvm.src/tools/clang/include
>>> -Itools/clang/include -Iinclude
>>> -I/home/buildslave/buildslave1a/clang-with-lto-ubuntu/llvm.src/include
>>> -I/home/buildslave/buildslave1a/clang-with-lto-ubuntu/llvm.src/utils/unittest/googletest/include
>>> -I/home/buildslave/buildslave1a/clang-with-lto-ubuntu/llvm.src/utils/unittest/googlemock/include
>>> -fPIC -fvisibility-inlines-hidden -std=c++11 -Wall -W -Wno-unused-parameter
>>> -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic
>>> -Wno-long-long -Wno-maybe-uninitialized -Wdelete-non-virtual-dtor
>>> -Wno-comment -ffunction-sections -fdata-sections -fno-common
>>> -Woverloaded-virtual -fno-strict-aliasing -O3 -DNDEBUG
>>> -Wno-variadic-macros -fno-exceptions -fno-rtti -MD -MT
>>> tools/clang/unittests/AST/CMakeFiles/ASTTests.dir/CommentTextTest.cpp.o -MF
>>> tools/clang/unittests/AST/CMakeFiles/ASTTests.dir/CommentTextTest.cpp.o.d
>>> -o tools/clang/unittests/AST/CMakeFiles/ASTTests.dir/CommentTextTest.cpp.o
>>> -c
>>> /home/buildslave/buildslave1a/clang-with-lto-ubuntu/llvm.src/tools/clang/unittests/AST/CommentTextTest.cpp
>>> /home/buildslave/buildslave1a/clang-with-lto-ubuntu/llvm.src/tools/clang/unittests/AST/CommentTextTest.cpp:62:1:
>>> error: unterminated raw string
>>>  R"cpp(
>>>  ^
>>> . . .
>>>
>>> Please have a look?
>>>
>>> The builder was already red and did not send notifications.
>>>
>>> Thanks
>>>
>>> Galina
>>>
>>>
>>>
>>> On Wed, May 16, 2018 at 5:30 AM, Ilya Biryukov via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
 Author: ibiryukov
 Date: Wed May 16 05:30:09 2018
 New Revision: 332458

 URL: http://llvm.org/viewvc/llvm-project?rev=332458=rev
 Log:
 [AST] Added a helper to extract a user-friendly text of a comment.

 Summary:
 The helper is used in clangd for documentation shown in code completion
 and storing the docs in the symbols. See D45999.

 This patch reuses the code of the Doxygen comment lexer, disabling the
 bits that do command and html tag parsing.
 The new helper works on all comments, including non-doxygen comments.
 However, it does not understand or transform any doxygen directives,
 i.e. cannot extract brief text, etc.

 Reviewers: sammccall, hokein, ioeric

 Reviewed By: ioeric

 Subscribers: mgorny, cfe-commits

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

 Added:
 cfe/trunk/unittests/AST/CommentTextTest.cpp
 Modified:
 cfe/trunk/include/clang/AST/CommentLexer.h
 cfe/trunk/include/clang/AST/RawCommentList.h
 cfe/trunk/lib/AST/CommentLexer.cpp
 cfe/trunk/lib/AST/RawCommentList.cpp
 cfe/trunk/unittests/AST/CMakeLists.txt

 Modified: cfe/trunk/include/clang/AST/CommentLexer.h
 URL:
 http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/CommentLexer.h?rev=332458=332457=332458=diff

 ==
 --- cfe/trunk/include/clang/AST/CommentLexer.h (original)
 +++ cfe/trunk/include/clang/AST/CommentLexer.h Wed May 16 05:30:09 2018
 @@ -281,6 +281,11 @@ private:
/// command, including command marker.
SmallString<16> VerbatimBlockEndCommandName;

 +  /// If true, the commands, html tags, etc will be parsed and
 reported as
 +  /// separate tokens inside the comment body. If false, the comment
 text will
 +  /// be parsed into text and newline tokens.
 +  bool ParseCommands;
 +
/// Given a character reference name (e.g., "lt"), return the
 character that
/// it 

r332587 - [libclang] Allow skipping function bodies in preamble only

[Ivan Donchevskii via cfe-commits]

Author: yvvan
Date: Thu May 17 02:24:37 2018
New Revision: 332587

URL: http://llvm.org/viewvc/llvm-project?rev=332587=rev
Log:
[libclang] Allow skipping function bodies in preamble only

Second attempt. Fix line endings and warning.

As an addition to CXTranslationUnit_SkipFunctionBodies, provide the
new option CXTranslationUnit_LimitSkipFunctionBodiesToPreamble,
which constraints the skipping of functions bodies to the preamble
only. Function bodies in the main file are not affected if this
option is set.

Skipping function bodies only in the preamble is what clangd already
does and the introduced flag implements it for libclang clients.

Patch by Nikolai Kosjar.

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

Added:
cfe/trunk/test/Parser/skip-function-bodies.h
Modified:
cfe/trunk/include/clang-c/Index.h
cfe/trunk/include/clang/Frontend/ASTUnit.h
cfe/trunk/lib/Frontend/ASTUnit.cpp
cfe/trunk/test/Parser/skip-function-bodies.mm
cfe/trunk/tools/c-index-test/c-index-test.c
cfe/trunk/tools/libclang/CIndex.cpp

Modified: cfe/trunk/include/clang-c/Index.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang-c/Index.h?rev=332587=332586=332587=diff
==
--- cfe/trunk/include/clang-c/Index.h (original)
+++ cfe/trunk/include/clang-c/Index.h Thu May 17 02:24:37 2018
@@ -1324,7 +1324,15 @@ enum CXTranslationUnit_Flags {
   /**
* Sets the preprocessor in a mode for parsing a single file only.
*/
-  CXTranslationUnit_SingleFileParse = 0x400
+  CXTranslationUnit_SingleFileParse = 0x400,
+
+  /**
+   * \brief Used in combination with CXTranslationUnit_SkipFunctionBodies to
+   * constrain the skipping of function bodies to the preamble.
+   *
+   * The function bodies of the main file are not skipped.
+   */
+  CXTranslationUnit_LimitSkipFunctionBodiesToPreamble = 0x800
 };
 
 /**

Modified: cfe/trunk/include/clang/Frontend/ASTUnit.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/ASTUnit.h?rev=332587=332586=332587=diff
==
--- cfe/trunk/include/clang/Frontend/ASTUnit.h (original)
+++ cfe/trunk/include/clang/Frontend/ASTUnit.h Thu May 17 02:24:37 2018
@@ -81,6 +81,9 @@ class FileSystem;
 
 } // namespace vfs
 
+/// \brief Enumerates the available scopes for skipping function bodies.
+enum class SkipFunctionBodiesScope { None, Preamble, PreambleAndMainFile };
+
 /// Utility class for loading a ASTContext from an AST file.
 class ASTUnit {
 public:
@@ -348,6 +351,9 @@ private:
   /// inconsistent state, and is not safe to free.
   unsigned UnsafeToFree : 1;
 
+  /// \brief Enumerator specifying the scope for skipping function bodies.
+  SkipFunctionBodiesScope SkipFunctionBodies = SkipFunctionBodiesScope::None;
+
   /// Cache any "global" code-completion results, so that we can avoid
   /// recomputing them with each completion.
   void CacheCodeCompletionResults();
@@ -363,7 +369,7 @@ private:
 
   std::unique_ptr getMainBufferWithPrecompiledPreamble(
   std::shared_ptr PCHContainerOps,
-  const CompilerInvocation ,
+  CompilerInvocation ,
   IntrusiveRefCntPtr VFS, bool AllowRebuild = true,
   unsigned MaxLines = 0);
   void RealizeTopLevelDeclsFromPreamble();
@@ -801,9 +807,11 @@ public:
   TranslationUnitKind TUKind = TU_Complete,
   bool CacheCodeCompletionResults = false,
   bool IncludeBriefCommentsInCodeCompletion = false,
-  bool AllowPCHWithCompilerErrors = false, bool SkipFunctionBodies = false,
-  bool SingleFileParse = false,
-  bool UserFilesAreVolatile = false, bool ForSerialization = false,
+  bool AllowPCHWithCompilerErrors = false,
+  SkipFunctionBodiesScope SkipFunctionBodies =
+  SkipFunctionBodiesScope::None,
+  bool SingleFileParse = false, bool UserFilesAreVolatile = false,
+  bool ForSerialization = false,
   llvm::Optional ModuleFormat = llvm::None,
   std::unique_ptr *ErrAST = nullptr,
   IntrusiveRefCntPtr VFS = nullptr);

Modified: cfe/trunk/lib/Frontend/ASTUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ASTUnit.cpp?rev=332587=332586=332587=diff
==
--- cfe/trunk/lib/Frontend/ASTUnit.cpp (original)
+++ cfe/trunk/lib/Frontend/ASTUnit.cpp Thu May 17 02:24:37 2018
@@ -1271,7 +1271,7 @@ makeStandaloneDiagnostic(const LangOptio
 std::unique_ptr
 ASTUnit::getMainBufferWithPrecompiledPreamble(
 std::shared_ptr PCHContainerOps,
-const CompilerInvocation ,
+CompilerInvocation ,
 IntrusiveRefCntPtr VFS, bool AllowRebuild,
 unsigned MaxLines) {
   auto MainFilePath =
@@ -1338,9 +1338,18 @@ ASTUnit::getMainBufferWithPrecompiledPre
 SimpleTimer PreambleTimer(WantTiming);
 PreambleTimer.setOutput("Precompiling preamble");
 
+const bool PreviousSkipFunctionBodies =
+ 

[PATCH] D46958: [ASTImporter] Fix missing implict CXXRecordDecl

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In https://reviews.llvm.org/D46958#1101570, @a.sidorin wrote:

> So, we fail to add injected name to a CXXRecordDecl that has a described 
> class template?


Yes, we failed to import the implicit `CXXRecordDecl`.
Here is how it looked before the fix:

  From:
  ClassTemplateDecl 0xecae28  line:3:14 declToImport
  |-TemplateTypeParmDecl 0xecacd8  col:26 typename depth 0 
index 0 U
  `-CXXRecordDecl 0xecad90  line:3:14 struct declToImport 
definition
|-DefinitionData empty aggregate standard_layout trivially_copyable pod 
trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
| |-DefaultConstructor exists trivial constexpr needs_implicit 
defaulted_is_constexpr
| |-CopyConstructor simple trivial has_const_param needs_implicit 
implicit_has_const_param
| |-MoveConstructor exists simple trivial needs_implicit
| |-CopyAssignment trivial has_const_param needs_implicit 
implicit_has_const_param
| |-MoveAssignment exists simple trivial needs_implicit
| `-Destructor simple irrelevant trivial needs_implicit
`-CXXRecordDecl 0xecb0a0  col:14 implicit struct declToImport
  To:
  ClassTemplateDecl 0xf97cb0  col:14 declToImport
  |-TemplateTypeParmDecl 0xf97c00  col:26 typename depth 0 
index 0 U
  `-CXXRecordDecl 0xf97a48  col:14 struct declToImport 
definition
`-DefinitionData pass_in_registers empty aggregate standard_layout 
trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor 
can_const_default_init
  |-DefaultConstructor exists trivial constexpr needs_implicit 
defaulted_is_constexpr
  |-CopyConstructor simple trivial has_const_param needs_implicit 
implicit_has_const_param
  |-MoveConstructor exists simple trivial needs_implicit
  |-CopyAssignment trivial has_const_param needs_implicit 
implicit_has_const_param
  |-MoveAssignment exists simple trivial needs_implicit
  `-Destructor simple irrelevant trivial needs_implicit

Thanks for the review.


Repository:
  rC Clang

https://reviews.llvm.org/D46958



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

r332586 - [Frontend] Avoid running plugins during code completion parse

[Ivan Donchevskii via cfe-commits]

Author: yvvan
Date: Thu May 17 02:21:07 2018
New Revision: 332586

URL: http://llvm.org/viewvc/llvm-project?rev=332586=rev
Log:
[Frontend] Avoid running plugins during code completion parse

Second attempt. Proper line endings.

The parsing that is done for code completion is a special case that will
discard any generated diagnostics, so avoid running plugins for this
case in the first place to avoid performance penalties due to the
plugins.

A scenario for this is for example libclang with extra plugins like tidy.

Patch by Nikolai Kosjar

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

Added:
cfe/trunk/test/Index/complete-and-plugins.c
Modified:
cfe/trunk/lib/Frontend/FrontendAction.cpp

Modified: cfe/trunk/lib/Frontend/FrontendAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/FrontendAction.cpp?rev=332586=332585=332586=diff
==
--- cfe/trunk/lib/Frontend/FrontendAction.cpp (original)
+++ cfe/trunk/lib/Frontend/FrontendAction.cpp Thu May 17 02:21:07 2018
@@ -153,6 +153,10 @@ FrontendAction::CreateWrappedASTConsumer
   if (FrontendPluginRegistry::begin() == FrontendPluginRegistry::end())
 return Consumer;
 
+  // If this is a code completion run, avoid invoking the plugin consumers
+  if (CI.hasCodeCompletionConsumer())
+return Consumer;
+
   // Collect the list of plugins that go before the main action (in Consumers)
   // or after it (in AfterConsumers)
   std::vector Consumers;

Added: cfe/trunk/test/Index/complete-and-plugins.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/complete-and-plugins.c?rev=332586=auto
==
--- cfe/trunk/test/Index/complete-and-plugins.c (added)
+++ cfe/trunk/test/Index/complete-and-plugins.c Thu May 17 02:21:07 2018
@@ -0,0 +1,6 @@
+// RUN: c-index-test -code-completion-at=%s:7:1 -load 
%llvmshlibdir/PrintFunctionNames%pluginext -add-plugin print-fns %s | FileCheck 
%s
+// REQUIRES: plugins, examples
+// CHECK: macro definition:{{.*}}
+// CHECK-NOT: top-level-decl: "x"
+
+void x();


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

r332585 - Revert https://reviews.llvm.org/D46050 and https://reviews.llvm.org/D45815

[Ivan Donchevskii via cfe-commits]

Author: yvvan
Date: Thu May 17 02:15:22 2018
New Revision: 332585

URL: http://llvm.org/viewvc/llvm-project?rev=332585=rev
Log:
Revert https://reviews.llvm.org/D46050 and https://reviews.llvm.org/D45815

Windows line endings.
Requires proper resubmission.

Removed:
cfe/trunk/test/Index/complete-and-plugins.c
cfe/trunk/test/Parser/skip-function-bodies.h
Modified:
cfe/trunk/include/clang-c/Index.h
cfe/trunk/include/clang/Frontend/ASTUnit.h
cfe/trunk/lib/Frontend/ASTUnit.cpp
cfe/trunk/lib/Frontend/FrontendAction.cpp
cfe/trunk/test/Parser/skip-function-bodies.mm
cfe/trunk/tools/c-index-test/c-index-test.c
cfe/trunk/tools/libclang/CIndex.cpp

Modified: cfe/trunk/include/clang-c/Index.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang-c/Index.h?rev=332585=332584=332585=diff
==
--- cfe/trunk/include/clang-c/Index.h (original)
+++ cfe/trunk/include/clang-c/Index.h Thu May 17 02:15:22 2018
@@ -1321,21 +1321,13 @@ enum CXTranslationUnit_Flags {
*/
   CXTranslationUnit_KeepGoing = 0x200,
 
-  /**
-   * Sets the preprocessor in a mode for parsing a single file only.
-   */
-  CXTranslationUnit_SingleFileParse = 0x400,
-
-  /**
-   * \brief Used in combination with CXTranslationUnit_SkipFunctionBodies to
-   * constrain the skipping of function bodies to the preamble.
-   *
-   * The function bodies of the main file are not skipped.
-   */
-  CXTranslationUnit_LimitSkipFunctionBodiesToPreamble = 0x800,
-};
-
-/**
+  /**
+   * Sets the preprocessor in a mode for parsing a single file only.
+   */
+  CXTranslationUnit_SingleFileParse = 0x400
+};
+
+/**
  * Returns the set of flags that is suitable for parsing a translation
  * unit that is being edited.
  *

Modified: cfe/trunk/include/clang/Frontend/ASTUnit.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/ASTUnit.h?rev=332585=332584=332585=diff
==
--- cfe/trunk/include/clang/Frontend/ASTUnit.h (original)
+++ cfe/trunk/include/clang/Frontend/ASTUnit.h Thu May 17 02:15:22 2018
@@ -78,15 +78,12 @@ class TargetInfo;
 namespace vfs {
 
 class FileSystem;
-
-} // namespace vfs
-
-/// \brief Enumerates the available scopes for skipping function bodies.
-enum class SkipFunctionBodiesScope { None, Preamble, PreambleAndMainFile };
-
-/// Utility class for loading a ASTContext from an AST file.
-class ASTUnit {
-public:
+
+} // namespace vfs
+
+/// Utility class for loading a ASTContext from an AST file.
+class ASTUnit {
+public:
   struct StandaloneFixIt {
 std::pair RemoveRange;
 std::pair InsertFromRange;
@@ -348,15 +345,12 @@ private:
   unsigned CurrentTopLevelHashValue = 0;
   
   /// Bit used by CIndex to mark when a translation unit may be in an
-  /// inconsistent state, and is not safe to free.
-  unsigned UnsafeToFree : 1;
-
-  /// \brief Enumerator specifying the scope for skipping function bodies.
-  SkipFunctionBodiesScope SkipFunctionBodies = SkipFunctionBodiesScope::None;
-
-  /// Cache any "global" code-completion results, so that we can avoid
-  /// recomputing them with each completion.
-  void CacheCodeCompletionResults();
+  /// inconsistent state, and is not safe to free.
+  unsigned UnsafeToFree : 1;
+
+  /// Cache any "global" code-completion results, so that we can avoid
+  /// recomputing them with each completion.
+  void CacheCodeCompletionResults();
   
   /// Clear out and deallocate 
   void ClearCachedCompletionResults();
@@ -366,13 +360,13 @@ private:
   bool Parse(std::shared_ptr PCHContainerOps,
  std::unique_ptr OverrideMainBuffer,
  IntrusiveRefCntPtr VFS);
-
-  std::unique_ptr getMainBufferWithPrecompiledPreamble(
-  std::shared_ptr PCHContainerOps,
-  CompilerInvocation ,
-  IntrusiveRefCntPtr VFS, bool AllowRebuild = true,
-  unsigned MaxLines = 0);
-  void RealizeTopLevelDeclsFromPreamble();
+
+  std::unique_ptr getMainBufferWithPrecompiledPreamble(
+  std::shared_ptr PCHContainerOps,
+  const CompilerInvocation ,
+  IntrusiveRefCntPtr VFS, bool AllowRebuild = true,
+  unsigned MaxLines = 0);
+  void RealizeTopLevelDeclsFromPreamble();
 
   /// Transfers ownership of the objects (like SourceManager) from
   /// \param CI to this ASTUnit.
@@ -804,17 +798,15 @@ public:
   ArrayRef RemappedFiles = None,
   bool RemappedFilesKeepOriginalName = true,
   unsigned PrecompilePreambleAfterNParses = 0,
-  TranslationUnitKind TUKind = TU_Complete,
-  bool CacheCodeCompletionResults = false,
-  bool IncludeBriefCommentsInCodeCompletion = false,
-  bool AllowPCHWithCompilerErrors = false,
-  SkipFunctionBodiesScope SkipFunctionBodies =
-  SkipFunctionBodiesScope::None,
-  bool SingleFileParse = false, bool UserFilesAreVolatile = false,
-  bool ForSerialization = false,
- 

[PATCH] D46835: [ASTImporter] Do not try to remove invisible Decls from DeclContext

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 147274.
martong added a comment.

Addressing review comments.


Repository:
  rC Clang

https://reviews.llvm.org/D46835

Files:
  lib/AST/DeclBase.cpp
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1797,5 +1797,38 @@
  compoundStmt(has(callExpr(has(unresolvedMemberExpr());
 }
 
+struct DeclContextTest : ASTImporterTestBase {};
+
+TEST_P(DeclContextTest, removeDeclOfClassTemplateSpecialization) {
+  Decl *TU = getTuDecl(
+  R"(
+  namespace NS {
+
+  template 
+  struct S {};
+  template struct S;
+
+  inline namespace INS {
+template 
+struct S {};
+template struct S;
+  }
+
+  }
+  )", Lang_CXX11, "input0.cc");
+  auto *NS = FirstDeclMatcher().match(
+  TU, namespaceDecl());
+  auto *Spec = FirstDeclMatcher().match(
+  TU, classTemplateSpecializationDecl());
+  ASSERT_TRUE(NS->containsDecl(Spec));
+
+  NS->removeDecl(Spec);
+  EXPECT_FALSE(NS->containsDecl(Spec));
+}
+
+INSTANTIATE_TEST_CASE_P(
+ParameterizedTests, DeclContextTest,
+::testing::Values(ArgVector(), ArgVector{"-fdelayed-template-parsing"}),);
+
 } // end namespace ast_matchers
 } // end namespace clang
Index: lib/AST/DeclBase.cpp
===
--- lib/AST/DeclBase.cpp
+++ lib/AST/DeclBase.cpp
@@ -1350,6 +1350,32 @@
   (D->NextInContextAndBits.getPointer() || D == LastDecl));
 }
 
+/// shouldBeHidden - Determine whether a declaration which was declared
+/// within its semantic context should be invisible to qualified name lookup.
+static bool shouldBeHidden(NamedDecl *D) {
+  // Skip unnamed declarations.
+  if (!D->getDeclName())
+return true;
+
+  // Skip entities that can't be found by name lookup into a particular
+  // context.
+  if ((D->getIdentifierNamespace() == 0 && !isa(D)) ||
+  D->isTemplateParameter())
+return true;
+
+  // Skip template specializations.
+  // FIXME: This feels like a hack. Should DeclarationName support
+  // template-ids, or is there a better way to keep specializations
+  // from being visible?
+  if (isa(D))
+return true;
+  if (auto *FD = dyn_cast(D))
+if (FD->isFunctionTemplateSpecialization())
+  return true;
+
+  return false;
+}
+
 void DeclContext::removeDecl(Decl *D) {
   assert(D->getLexicalDeclContext() == this &&
  "decl being removed from non-lexical context");
@@ -1372,16 +1398,22 @@
   }
 }
   }
-  
+
   // Mark that D is no longer in the decl chain.
   D->NextInContextAndBits.setPointer(nullptr);
 
   // Remove D from the lookup table if necessary.
   if (isa(D)) {
 auto *ND = cast(D);
 
+// Do not try to remove the declaration if that is invisible to qualified
+// lookup.  E.g. template specializations are skipped.
+if (shouldBeHidden(ND))
+  return;
+
 // Remove only decls that have a name
-if (!ND->getDeclName()) return;
+if (!ND->getDeclName())
+  return;
 
 auto *DC = D->getDeclContext();
 do {
@@ -1438,32 +1470,6 @@
 makeDeclVisibleInContextWithFlags(ND, true, true);
 }
 
-/// shouldBeHidden - Determine whether a declaration which was declared
-/// within its semantic context should be invisible to qualified name lookup.
-static bool shouldBeHidden(NamedDecl *D) {
-  // Skip unnamed declarations.
-  if (!D->getDeclName())
-return true;
-
-  // Skip entities that can't be found by name lookup into a particular
-  // context.
-  if ((D->getIdentifierNamespace() == 0 && !isa(D)) ||
-  D->isTemplateParameter())
-return true;
-
-  // Skip template specializations.
-  // FIXME: This feels like a hack. Should DeclarationName support
-  // template-ids, or is there a better way to keep specializations
-  // from being visible?
-  if (isa(D))
-return true;
-  if (auto *FD = dyn_cast(D))
-if (FD->isFunctionTemplateSpecialization())
-  return true;
-
-  return false;
-}
-
 /// buildLookup - Build the lookup data structure with all of the
 /// declarations in this DeclContext (and any other contexts linked
 /// to it or transparent contexts nested within it) and return it.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46940: [ASTImporter] make sure that ACtx::getParents still works

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl updated this revision to Diff 147268.
r.stahl marked 2 inline comments as done.
r.stahl added a comment.

addressed review comments


https://reviews.llvm.org/D46940

Files:
  include/clang/AST/ASTContext.h
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1431,6 +1431,36 @@
   MatchVerifier{}.match(To->getTranslationUnitDecl(), Pattern));
 }
 
+TEST_P(ASTImporterTestBase, ValidParents) {
+  // Create parent cache in To context.
+  Decl *PreTU = getTuDecl("struct S {};", Lang_CXX, "pre.cc");
+  RecordDecl *FromRD = FirstDeclMatcher().match(PreTU, recordDecl());
+  auto ToRD = cast(Import(FromRD, Lang_CXX));
+  auto  = ToAST->getASTContext();
+  ToACtx.getParents(*ToRD);
+
+  Decl *FromTU = getTuDecl("void f() {}", Lang_CXX);
+  CompoundStmt *FromCS = FirstDeclMatcher().match(FromTU,
+compoundStmt());
+  FunctionDecl *FromFD = FirstDeclMatcher().match(FromTU,
+functionDecl());
+
+  auto FromPs = FromFD->getASTContext().getParents(*FromCS);
+  ASSERT_TRUE(!FromPs.empty());
+  auto FromP = FromPs[0].get();
+  EXPECT_EQ(FromP, FromFD);
+
+  auto ToFD = cast(Import(FromFD, Lang_CXX));
+
+  Decl *ToTU = ToACtx.getTranslationUnitDecl();
+  auto ToCS = FirstDeclMatcher().match(ToTU, compoundStmt());
+
+  auto ToPs = ToACtx.getParents(*ToCS);
+  ASSERT_TRUE(!ToPs.empty());
+  auto ToP = ToPs[0].get();
+  EXPECT_EQ(ToP, ToFD);
+}
+
 INSTANTIATE_TEST_CASE_P(
 ParameterizedTests, ASTImporterTestBase,
 ::testing::Values(ArgVector(), ArgVector{"-fdelayed-template-parsing"}),);
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -6686,6 +6686,8 @@
   if (!ToD)
 return nullptr;
 
+  ToContext.invalidateParents();
+
   // Record the imported declaration.
   ImportedDecls[FromD] = ToD;
   ToD->IdentifierNamespace = FromD->IdentifierNamespace;
@@ -6762,13 +6764,17 @@
   llvm::DenseMap::iterator Pos = ImportedStmts.find(FromS);
   if (Pos != ImportedStmts.end())
 return Pos->second;
-  
+
   // Import the type
   ASTNodeImporter Importer(*this);
   Stmt *ToS = Importer.Visit(FromS);
   if (!ToS)
 return nullptr;
 
+  // FIXME: We could add a separate function that assumes parents have already
+  // been invalidated to avoid repeatedly calling this.
+  ToContext.invalidateParents();
+
   // Record the imported declaration.
   ImportedStmts[FromS] = ToS;
   return ToS;
@@ -6779,52 +6785,60 @@
 return nullptr;
 
   NestedNameSpecifier *prefix = Import(FromNNS->getPrefix());
+  NestedNameSpecifier *ToNNS = nullptr;
 
   switch (FromNNS->getKind()) {
   case NestedNameSpecifier::Identifier:
 if (IdentifierInfo *II = Import(FromNNS->getAsIdentifier())) {
-  return NestedNameSpecifier::Create(ToContext, prefix, II);
+  ToNNS = NestedNameSpecifier::Create(ToContext, prefix, II);
 }
-return nullptr;
+break;
 
   case NestedNameSpecifier::Namespace:
 if (auto *NS = 
 cast_or_null(Import(FromNNS->getAsNamespace( {
-  return NestedNameSpecifier::Create(ToContext, prefix, NS);
+  ToNNS = NestedNameSpecifier::Create(ToContext, prefix, NS);
 }
-return nullptr;
+break;
 
   case NestedNameSpecifier::NamespaceAlias:
 if (auto *NSAD = 
   cast_or_null(Import(FromNNS->getAsNamespaceAlias( {
-  return NestedNameSpecifier::Create(ToContext, prefix, NSAD);
+  ToNNS = NestedNameSpecifier::Create(ToContext, prefix, NSAD);
 }
-return nullptr;
+break;
 
   case NestedNameSpecifier::Global:
-return NestedNameSpecifier::GlobalSpecifier(ToContext);
+ToNNS = NestedNameSpecifier::GlobalSpecifier(ToContext);
+break;
 
   case NestedNameSpecifier::Super:
 if (auto *RD =
 cast_or_null(Import(FromNNS->getAsRecordDecl( {
-  return NestedNameSpecifier::SuperSpecifier(ToContext, RD);
+  ToNNS = NestedNameSpecifier::SuperSpecifier(ToContext, RD);
 }
-return nullptr;
+break;
 
   case NestedNameSpecifier::TypeSpec:
   case NestedNameSpecifier::TypeSpecWithTemplate: {
   QualType T = Import(QualType(FromNNS->getAsType(), 0u));
   if (!T.isNull()) {
 bool bTemplate = FromNNS->getKind() == 
  NestedNameSpecifier::TypeSpecWithTemplate;
-return NestedNameSpecifier::Create(ToContext, prefix, 
+ToNNS = NestedNameSpecifier::Create(ToContext, prefix, 
bTemplate, T.getTypePtr());
   }
+  break;
 }
-  return nullptr;
+
+  default:
+llvm_unreachable("Invalid nested name specifier kind");
   }
 
-  llvm_unreachable("Invalid nested name specifier 

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

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Sorry, a few more things and we're good to go.




Comment at: include/clang/Driver/CC1Options.td:449
+def code_completion_include_fixits : Flag<["-"], 
"code-completion-include-fixits">,
+  HelpText<"Include code completion results which require having small fix-its 
applied.">;
 def disable_free : Flag<["-"], "disable-free">,

NIT: maybe simplify to "which require small fix-its"?



Comment at: lib/Frontend/CompilerInvocation.cpp:1541
+  Opts.CodeCompleteOpts.IncludeFixIts
+= Args.hasArg(OPT_code_completion_include_fixits);
 

Following naming convention of other options, maybe use 
`OPT_code_completion_with_fixits`? (i.e. none of them start with include)



Comment at: lib/Sema/CodeCompleteConsumer.cpp:559
+const char *Begin =
+SemaRef.SourceMgr.getCharacterData(FixIt.RemoveRange.getBegin());
+const char *End =

Unfortunately, that might not work for some ranges, see docs of 
`CharSourceRange`:
```
/// In the token range case, the
/// size of the last token must be measured to determine the actual end of the
/// range.
```

The `TextDiagnostic::emitParseableFixits` handles it, I suggest we do it 
similarly:
```
FixItHint*I = //;
SourceLocation BLoc = I->RemoveRange.getBegin();
SourceLocation ELoc = I->RemoveRange.getEnd();

std::pair BInfo = SM.getDecomposedLoc(BLoc);
std::pair EInfo = SM.getDecomposedLoc(ELoc);

// Adjust for token ranges.
if (I->RemoveRange.isTokenRange())
  EInfo.second += Lexer::MeasureTokenLength(ELoc, SM, LangOpts);

// We specifically do not do word-wrapping or tab-expansion here,
// because this is supposed to be easy to parse.
PresumedLoc PLoc = SM.getPresumedLoc(BLoc);
if (PLoc.isInvalid())
  break;

OS << "fix-it:\"";
OS.write_escaped(PLoc.getFilename());
OS << "\":{" << SM.getLineNumber(BInfo.first, BInfo.second)
  << ':' << SM.getColumnNumber(BInfo.first, BInfo.second)
  << '-' << SM.getLineNumber(EInfo.first, EInfo.second)
  << ':' << SM.getColumnNumber(EInfo.first, EInfo.second)
  << "}:\"";
OS.write_escaped(I->CodeToInsert);
OS << "\"\n";
```



Comment at: lib/Sema/SemaCodeComplete.cpp:4109
+  if (CodeCompleter->includeFixIts()) {
+const SourceRange OpRange(OpLoc, OpLoc.getLocWithOffset(IsArrow ? 2 : 1));
+CompletionSucceded =

I'd use token ranges here to avoid assumptions about sizes of tokens, e.g. 
`CreateReplacemen(CharSourceRange::getTokenRange(OpLoc, OpLoc), IsArrow ? '.' : 
'->')`
There are complicated cases like `\` that end of the line and macros and it's 
definitely better to use an abstraction that hides those cases.


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] D44100: [ASTImporter] Reorder fields after structure import is finished

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Hi Aleksei,

I am OK with this, I just have a little concern about friend declarations. 
Since https://reviews.llvm.org/D32947 ( [ASTImporter] FriendDecl importing 
improvements ) records' structural equivalency depends on the order of their 
friend declarations.
Anyway, I think friend related issues can be addressed in a separate patch.

What is a bit more concerning is that `*ToField` can be null and we should 
handle that.
This could happen if lookup finds the type of the field, but for some reason 
(e.g. there is an error in structural eq, or the redecl chain is incorrect) it 
turns out to be non-equivalent with the type of the FromField:

  for (auto *FoundDecl : FoundDecls) {
if (auto *FoundField = dyn_cast(FoundDecl)) {
  // For anonymous fields, match up by index.
  if (!Name && getFieldIndex(D) != getFieldIndex(FoundField))
continue;
  
  if (Importer.IsStructurallyEquivalent(D->getType(),
FoundField->getType())) {
Importer.Imported(D, FoundField);
return FoundField;
  }
  
  Importer.ToDiag(Loc, diag::err_odr_field_type_inconsistent)
<< Name << D->getType() << FoundField->getType();
  Importer.ToDiag(FoundField->getLocation(), diag::note_odr_value_here)
<< FoundField->getType();
  return nullptr;
}
  }






Comment at: lib/AST/ASTImporter.cpp:1025
+  // type import can depend on them.
+  const RecordDecl *FromRD = dyn_cast(FromDC);
+  if (!FromRD)

Could use `auto` here, to avoid redundant type specification.



Comment at: lib/AST/ASTImporter.cpp:1029
+
+  RecordDecl *ToRD = cast(Importer.Import(cast(FromDC)));
+

Can't we just import the `FromRD`, why we need that cast at the end? 
`auto *ToRD = cast(Importer.Import(FromRD)));` 



Comment at: lib/AST/ASTImporter.cpp:1032
+  for (auto *FromField : FromRD->fields()) {
+Decl *ToField = Importer.GetAlreadyImportedOrNull(FromField);
+assert(ToRD == ToField->getDeclContext() && ToRD->containsDecl(ToField));

I think `ToField` can be a nullptr, and if so, then `ToField->getDeclContext()` 
is UB.
Same could happen at line 1040.
Perhaps we should have and explicit check about the nullptr value:
`if (!ToField) continue;`




Comment at: unittests/AST/ASTImporterTest.cpp:1022
 
+AST_MATCHER_P(RecordDecl, hasFieldOrder, std::vector, Order) {
+  size_t Index = 0;

The `hasFieldOrder` matcher is already existing.



Comment at: unittests/AST/ASTImporterTest.cpp:1034
+
+TEST(ImportDecl, ImportFieldOrder) {
+  MatchVerifier Verifier;

We already have this test, we just have to enable it.
`DISABLED_CXXRecordDeclFieldOrderShouldNotDependOnImportOrder`


Repository:
  rC Clang

https://reviews.llvm.org/D44100



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

[PATCH] D46879: [clang-format] Fix putting ObjC message arguments in one line for multiline receiver

[Jacek Olesiak via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL332582: [clang-format] Fix putting ObjC message arguments in 
one line for multiline… (authored by jolesiak, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D46879

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


Index: cfe/trunk/lib/Format/ContinuationIndenter.cpp
===
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp
@@ -1073,8 +1073,34 @@
   if (Current.isMemberAccess())
 State.Stack.back().StartOfFunctionCall =
 !Current.NextOperator ? 0 : State.Column;
-  if (Current.is(TT_SelectorName))
+  if (Current.is(TT_SelectorName) &&
+  !State.Stack.back().ObjCSelectorNameFound) {
 State.Stack.back().ObjCSelectorNameFound = true;
+
+// Reevaluate whether ObjC message arguments fit into one line.
+// If a receiver spans multiple lines, e.g.:
+//   [[object block:^{
+// return 42;
+//   }] a:42 b:42];
+// BreakBeforeParameter is calculated based on an incorrect assumption
+// (it is checked whether the whole expression fits into one line without
+// considering a line break inside a message receiver).
+if (Current.Previous && Current.Previous->closesScope() &&
+Current.Previous->MatchingParen &&
+Current.Previous->MatchingParen->Previous) {
+  const FormatToken  =
+  *Current.Previous->MatchingParen->Previous;
+  if (CurrentScopeOpener.is(TT_ObjCMethodExpr) &&
+  CurrentScopeOpener.MatchingParen) {
+int NecessarySpaceInLine =
+getLengthToMatchingParen(CurrentScopeOpener, State.Stack) +
+CurrentScopeOpener.TotalLength - Current.TotalLength - 1;
+if (State.Column + Current.ColumnWidth + NecessarySpaceInLine <=
+Style.ColumnLimit)
+  State.Stack.back().BreakBeforeParameter = false;
+  }
+}
+  }
   if (Current.is(TT_CtorInitializerColon) &&
   Style.BreakConstructorInitializers != FormatStyle::BCIS_AfterColon) {
 // Indent 2 from the column, so:
Index: cfe/trunk/unittests/Format/FormatTestObjC.cpp
===
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp
@@ -792,6 +792,35 @@
"  a = 42;\n"
"}];");
 
+  // Message receiver taking multiple lines.
+  Style.ColumnLimit = 20;
+  // Non-corner case.
+  verifyFormat("[[object block:^{\n"
+   "  return 42;\n"
+   "}] a:42 b:42];");
+  // Arguments just fit into one line.
+  verifyFormat("[[object block:^{\n"
+   "  return 42;\n"
+   "}] aaa:42 b:42];");
+  // Arguments just over a column limit.
+  verifyFormat("[[object block:^{\n"
+   "  return 42;\n"
+   "}] aaa:42\n"
+   "bb:42];");
+  // Non-corner case.
+  verifyFormat("[[object aaa:42\n"
+   "   b:42]\n"
+   "cc:42 d:42];");
+  // Arguments just fit into one line.
+  verifyFormat("[[object aaa:42\n"
+   "   b:42]\n"
+   "cc:42 d:42];");
+  // Arguments just over a column limit.
+  verifyFormat("[[object aaa:42\n"
+   "   b:42]\n"
+   "cc:42\n"
+   "dd:42];");
+
   Style.ColumnLimit = 70;
   verifyFormat(
   "void f() {\n"


Index: cfe/trunk/lib/Format/ContinuationIndenter.cpp
===
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp
@@ -1073,8 +1073,34 @@
   if (Current.isMemberAccess())
 State.Stack.back().StartOfFunctionCall =
 !Current.NextOperator ? 0 : State.Column;
-  if (Current.is(TT_SelectorName))
+  if (Current.is(TT_SelectorName) &&
+  !State.Stack.back().ObjCSelectorNameFound) {
 State.Stack.back().ObjCSelectorNameFound = true;
+
+// Reevaluate whether ObjC message arguments fit into one line.
+// If a receiver spans multiple lines, e.g.:
+//   [[object block:^{
+// return 42;
+//   }] a:42 b:42];
+// BreakBeforeParameter is calculated based on an incorrect assumption
+// (it is checked whether the whole expression fits into one line without
+// considering a line break inside a message receiver).
+if (Current.Previous && Current.Previous->closesScope() &&
+Current.Previous->MatchingParen &&
+Current.Previous->MatchingParen->Previous) {
+  const FormatToken  =
+  *Current.Previous->MatchingParen->Previous;
+  if (CurrentScopeOpener.is(TT_ObjCMethodExpr) &&
+  CurrentScopeOpener.MatchingParen) {
+int NecessarySpaceInLine =

[PATCH] D46879: [clang-format] Fix putting ObjC message arguments in one line for multiline receiver

[Jacek Olesiak via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC332582: [clang-format] Fix putting ObjC message arguments in 
one line for multiline… (authored by jolesiak, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D46879?vs=146818=147265#toc

Repository:
  rC Clang

https://reviews.llvm.org/D46879

Files:
  lib/Format/ContinuationIndenter.cpp
  unittests/Format/FormatTestObjC.cpp


Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -1073,8 +1073,34 @@
   if (Current.isMemberAccess())
 State.Stack.back().StartOfFunctionCall =
 !Current.NextOperator ? 0 : State.Column;
-  if (Current.is(TT_SelectorName))
+  if (Current.is(TT_SelectorName) &&
+  !State.Stack.back().ObjCSelectorNameFound) {
 State.Stack.back().ObjCSelectorNameFound = true;
+
+// Reevaluate whether ObjC message arguments fit into one line.
+// If a receiver spans multiple lines, e.g.:
+//   [[object block:^{
+// return 42;
+//   }] a:42 b:42];
+// BreakBeforeParameter is calculated based on an incorrect assumption
+// (it is checked whether the whole expression fits into one line without
+// considering a line break inside a message receiver).
+if (Current.Previous && Current.Previous->closesScope() &&
+Current.Previous->MatchingParen &&
+Current.Previous->MatchingParen->Previous) {
+  const FormatToken  =
+  *Current.Previous->MatchingParen->Previous;
+  if (CurrentScopeOpener.is(TT_ObjCMethodExpr) &&
+  CurrentScopeOpener.MatchingParen) {
+int NecessarySpaceInLine =
+getLengthToMatchingParen(CurrentScopeOpener, State.Stack) +
+CurrentScopeOpener.TotalLength - Current.TotalLength - 1;
+if (State.Column + Current.ColumnWidth + NecessarySpaceInLine <=
+Style.ColumnLimit)
+  State.Stack.back().BreakBeforeParameter = false;
+  }
+}
+  }
   if (Current.is(TT_CtorInitializerColon) &&
   Style.BreakConstructorInitializers != FormatStyle::BCIS_AfterColon) {
 // Indent 2 from the column, so:
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -792,6 +792,35 @@
"  a = 42;\n"
"}];");
 
+  // Message receiver taking multiple lines.
+  Style.ColumnLimit = 20;
+  // Non-corner case.
+  verifyFormat("[[object block:^{\n"
+   "  return 42;\n"
+   "}] a:42 b:42];");
+  // Arguments just fit into one line.
+  verifyFormat("[[object block:^{\n"
+   "  return 42;\n"
+   "}] aaa:42 b:42];");
+  // Arguments just over a column limit.
+  verifyFormat("[[object block:^{\n"
+   "  return 42;\n"
+   "}] aaa:42\n"
+   "bb:42];");
+  // Non-corner case.
+  verifyFormat("[[object aaa:42\n"
+   "   b:42]\n"
+   "cc:42 d:42];");
+  // Arguments just fit into one line.
+  verifyFormat("[[object aaa:42\n"
+   "   b:42]\n"
+   "cc:42 d:42];");
+  // Arguments just over a column limit.
+  verifyFormat("[[object aaa:42\n"
+   "   b:42]\n"
+   "cc:42\n"
+   "dd:42];");
+
   Style.ColumnLimit = 70;
   verifyFormat(
   "void f() {\n"


Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -1073,8 +1073,34 @@
   if (Current.isMemberAccess())
 State.Stack.back().StartOfFunctionCall =
 !Current.NextOperator ? 0 : State.Column;
-  if (Current.is(TT_SelectorName))
+  if (Current.is(TT_SelectorName) &&
+  !State.Stack.back().ObjCSelectorNameFound) {
 State.Stack.back().ObjCSelectorNameFound = true;
+
+// Reevaluate whether ObjC message arguments fit into one line.
+// If a receiver spans multiple lines, e.g.:
+//   [[object block:^{
+// return 42;
+//   }] a:42 b:42];
+// BreakBeforeParameter is calculated based on an incorrect assumption
+// (it is checked whether the whole expression fits into one line without
+// considering a line break inside a message receiver).
+if (Current.Previous && Current.Previous->closesScope() &&
+Current.Previous->MatchingParen &&
+Current.Previous->MatchingParen->Previous) {
+  const FormatToken  =
+  *Current.Previous->MatchingParen->Previous;
+  if (CurrentScopeOpener.is(TT_ObjCMethodExpr) &&
+  CurrentScopeOpener.MatchingParen) {
+int NecessarySpaceInLine =
+getLengthToMatchingParen(CurrentScopeOpener, 

r332582 - [clang-format] Fix putting ObjC message arguments in one line for multiline receiver

[Jacek Olesiak via cfe-commits]

Author: jolesiak
Date: Thu May 17 01:35:15 2018
New Revision: 332582

URL: http://llvm.org/viewvc/llvm-project?rev=332582=rev
Log:
[clang-format] Fix putting ObjC message arguments in one line for multiline 
receiver

Summary:
Currently BreakBeforeParameter is set to true everytime message receiver spans 
multiple lines, e.g.:
```
[[object block:^{
  return 42;
}] aa:42 bb:42];
```
will be formatted:
```
[[object block:^{
  return 42;
}] aa:42
   bb:42];
```
even though arguments could fit into one line. This change fixes this behavior.

Test Plan:
make -j12 FormatTests && tools/clang/unittests/Format/FormatTests

Reviewers: benhamilton, djasper

Reviewed By: benhamilton

Subscribers: klimek, cfe-commits

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

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

Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=332582=332581=332582=diff
==
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Thu May 17 01:35:15 2018
@@ -1073,8 +1073,34 @@ unsigned ContinuationIndenter::moveState
   if (Current.isMemberAccess())
 State.Stack.back().StartOfFunctionCall =
 !Current.NextOperator ? 0 : State.Column;
-  if (Current.is(TT_SelectorName))
+  if (Current.is(TT_SelectorName) &&
+  !State.Stack.back().ObjCSelectorNameFound) {
 State.Stack.back().ObjCSelectorNameFound = true;
+
+// Reevaluate whether ObjC message arguments fit into one line.
+// If a receiver spans multiple lines, e.g.:
+//   [[object block:^{
+// return 42;
+//   }] a:42 b:42];
+// BreakBeforeParameter is calculated based on an incorrect assumption
+// (it is checked whether the whole expression fits into one line without
+// considering a line break inside a message receiver).
+if (Current.Previous && Current.Previous->closesScope() &&
+Current.Previous->MatchingParen &&
+Current.Previous->MatchingParen->Previous) {
+  const FormatToken  =
+  *Current.Previous->MatchingParen->Previous;
+  if (CurrentScopeOpener.is(TT_ObjCMethodExpr) &&
+  CurrentScopeOpener.MatchingParen) {
+int NecessarySpaceInLine =
+getLengthToMatchingParen(CurrentScopeOpener, State.Stack) +
+CurrentScopeOpener.TotalLength - Current.TotalLength - 1;
+if (State.Column + Current.ColumnWidth + NecessarySpaceInLine <=
+Style.ColumnLimit)
+  State.Stack.back().BreakBeforeParameter = false;
+  }
+}
+  }
   if (Current.is(TT_CtorInitializerColon) &&
   Style.BreakConstructorInitializers != FormatStyle::BCIS_AfterColon) {
 // Indent 2 from the column, so:

Modified: cfe/trunk/unittests/Format/FormatTestObjC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestObjC.cpp?rev=332582=332581=332582=diff
==
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp Thu May 17 01:35:15 2018
@@ -792,6 +792,35 @@ TEST_F(FormatTestObjC, FormatObjCMethodE
"  a = 42;\n"
"}];");
 
+  // Message receiver taking multiple lines.
+  Style.ColumnLimit = 20;
+  // Non-corner case.
+  verifyFormat("[[object block:^{\n"
+   "  return 42;\n"
+   "}] a:42 b:42];");
+  // Arguments just fit into one line.
+  verifyFormat("[[object block:^{\n"
+   "  return 42;\n"
+   "}] aaa:42 b:42];");
+  // Arguments just over a column limit.
+  verifyFormat("[[object block:^{\n"
+   "  return 42;\n"
+   "}] aaa:42\n"
+   "bb:42];");
+  // Non-corner case.
+  verifyFormat("[[object aaa:42\n"
+   "   b:42]\n"
+   "cc:42 d:42];");
+  // Arguments just fit into one line.
+  verifyFormat("[[object aaa:42\n"
+   "   b:42]\n"
+   "cc:42 d:42];");
+  // Arguments just over a column limit.
+  verifyFormat("[[object aaa:42\n"
+   "   b:42]\n"
+   "cc:42\n"
+   "dd:42];");
+
   Style.ColumnLimit = 70;
   verifyFormat(
   "void f() {\n"


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

[PATCH] D40481: [libclang] Fix cursors for arguments of Subscript and Call operators

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan added a comment.

Tests run fine but the solution feels like a workaround.

Nevertheless if we are sure that extending the subscript/call operator range 
does not work properly or breaks too many other things than it's probably fine 
to have this workaround.


https://reviews.llvm.org/D40481



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

[PATCH] D45177: CStringChecker, check strlcpy/strlcat

[David CARLIER via Phabricator via cfe-commits]

devnexen added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:1560-1566
 // If the size is known to be zero, we're done.
 if (StateZeroSize && !StateNonZeroSize) {
   StateZeroSize = StateZeroSize->BindExpr(CE, LCtx, DstVal);
   C.addTransition(StateZeroSize);
   return;
 }
 

NoQ wrote:
> One more cornercase where the return value needs to be corrected. It'd be 
> great to de-duplicate this code to avoid similar problems in the future.
> 
> Test case:
> ```
> int foo(char *dst, const char *src) {
>   return strlcpy(dst, src, 0); // no-crash
> }
> ```
Thanks for the hint ! will do a separate "PR".


Repository:
  rC Clang

https://reviews.llvm.org/D45177



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

[PATCH] D45815: [libclang] Allow skipping function bodies in preamble only

[Ivan Donchevskii via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC332578: [libclang] Allow skipping function bodies in 
preamble only (authored by yvvan, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D45815?vs=145648=147253#toc

Repository:
  rC Clang

https://reviews.llvm.org/D45815

Files:
  include/clang-c/Index.h
  include/clang/Frontend/ASTUnit.h
  lib/Frontend/ASTUnit.cpp
  test/Parser/skip-function-bodies.h
  test/Parser/skip-function-bodies.mm
  tools/c-index-test/c-index-test.c
  tools/libclang/CIndex.cpp

Index: lib/Frontend/ASTUnit.cpp
===
--- lib/Frontend/ASTUnit.cpp
+++ lib/Frontend/ASTUnit.cpp
@@ -1268,13 +1268,13 @@
 /// \returns If the precompiled preamble can be used, returns a newly-allocated
 /// buffer that should be used in place of the main file when doing so.
 /// Otherwise, returns a NULL pointer.
-std::unique_ptr
-ASTUnit::getMainBufferWithPrecompiledPreamble(
-std::shared_ptr PCHContainerOps,
-const CompilerInvocation ,
-IntrusiveRefCntPtr VFS, bool AllowRebuild,
-unsigned MaxLines) {
-  auto MainFilePath =
+std::unique_ptr
+ASTUnit::getMainBufferWithPrecompiledPreamble(
+std::shared_ptr PCHContainerOps,
+CompilerInvocation ,
+IntrusiveRefCntPtr VFS, bool AllowRebuild,
+unsigned MaxLines) {
+  auto MainFilePath =
   PreambleInvocationIn.getFrontendOpts().Inputs[0].getFile();
   std::unique_ptr MainFileBuffer =
   getBufferForFileHandlingRemapping(PreambleInvocationIn, VFS.get(),
@@ -1335,15 +1335,24 @@
   );
 
 // We did not previously compute a preamble, or it can't be reused anyway.
-SimpleTimer PreambleTimer(WantTiming);
-PreambleTimer.setOutput("Precompiling preamble");
-
-llvm::ErrorOr NewPreamble = PrecompiledPreamble::Build(
-PreambleInvocationIn, MainFileBuffer.get(), Bounds, *Diagnostics, VFS,
-PCHContainerOps, /*StoreInMemory=*/false, Callbacks);
-if (NewPreamble) {
-  Preamble = std::move(*NewPreamble);
-  PreambleRebuildCounter = 1;
+SimpleTimer PreambleTimer(WantTiming);
+PreambleTimer.setOutput("Precompiling preamble");
+
+const bool PreviousSkipFunctionBodies =
+PreambleInvocationIn.getFrontendOpts().SkipFunctionBodies;
+if (SkipFunctionBodies == SkipFunctionBodiesScope::Preamble)
+  PreambleInvocationIn.getFrontendOpts().SkipFunctionBodies = true;
+
+llvm::ErrorOr NewPreamble = PrecompiledPreamble::Build(
+PreambleInvocationIn, MainFileBuffer.get(), Bounds, *Diagnostics, VFS,
+PCHContainerOps, /*StoreInMemory=*/false, Callbacks);
+
+PreambleInvocationIn.getFrontendOpts().SkipFunctionBodies =
+PreviousSkipFunctionBodies;
+
+if (NewPreamble) {
+  Preamble = std::move(*NewPreamble);
+  PreambleRebuildCounter = 1;
 } else {
   switch (static_cast(NewPreamble.getError().value())) {
   case BuildPreambleError::CouldntCreateTempFile:
@@ -1688,13 +1697,13 @@
 std::shared_ptr PCHContainerOps,
 IntrusiveRefCntPtr Diags, StringRef ResourceFilesPath,
 bool OnlyLocalDecls, bool CaptureDiagnostics,
-ArrayRef RemappedFiles, bool RemappedFilesKeepOriginalName,
-unsigned PrecompilePreambleAfterNParses, TranslationUnitKind TUKind,
-bool CacheCodeCompletionResults, bool IncludeBriefCommentsInCodeCompletion,
-bool AllowPCHWithCompilerErrors, bool SkipFunctionBodies,
-bool SingleFileParse, bool UserFilesAreVolatile, bool ForSerialization,
-llvm::Optional ModuleFormat, std::unique_ptr *ErrAST,
-IntrusiveRefCntPtr VFS) {
+ArrayRef RemappedFiles, bool RemappedFilesKeepOriginalName,
+unsigned PrecompilePreambleAfterNParses, TranslationUnitKind TUKind,
+bool CacheCodeCompletionResults, bool IncludeBriefCommentsInCodeCompletion,
+bool AllowPCHWithCompilerErrors, SkipFunctionBodiesScope SkipFunctionBodies,
+bool SingleFileParse, bool UserFilesAreVolatile, bool ForSerialization,
+llvm::Optional ModuleFormat, std::unique_ptr *ErrAST,
+IntrusiveRefCntPtr VFS) {
   assert(Diags.get() && "no DiagnosticsEngine was provided");
 
   SmallVector StoredDiagnostics;
@@ -1721,13 +1730,14 @@
   PPOpts.AllowPCHWithCompilerErrors = AllowPCHWithCompilerErrors;
   PPOpts.SingleFileParseMode = SingleFileParse;
 
-  // Override the resources path.
-  CI->getHeaderSearchOpts().ResourceDir = ResourceFilesPath;
-
-  CI->getFrontendOpts().SkipFunctionBodies = SkipFunctionBodies;
-
-  if (ModuleFormat)
-CI->getHeaderSearchOpts().ModuleFormat = ModuleFormat.getValue();
+  // Override the resources path.
+  CI->getHeaderSearchOpts().ResourceDir = ResourceFilesPath;
+
+  CI->getFrontendOpts().SkipFunctionBodies =
+  SkipFunctionBodies == SkipFunctionBodiesScope::PreambleAndMainFile;
+
+  if (ModuleFormat)
+CI->getHeaderSearchOpts().ModuleFormat = ModuleFormat.getValue();
 
   // Create the AST unit.
   std::unique_ptr AST;

r332578 - [libclang] Allow skipping function bodies in preamble only

[Ivan Donchevskii via cfe-commits]

Author: yvvan
Date: Thu May 17 00:31:29 2018
New Revision: 332578

URL: http://llvm.org/viewvc/llvm-project?rev=332578=rev
Log:
[libclang] Allow skipping function bodies in preamble only

As an addition to CXTranslationUnit_SkipFunctionBodies, provide the
new option CXTranslationUnit_LimitSkipFunctionBodiesToPreamble,
which constraints the skipping of functions bodies to the preamble
only. Function bodies in the main file are not affected if this
option is set.

Skipping function bodies only in the preamble is what clangd already
does and the introduced flag implements it for libclang clients.

Patch by Nikolai Kosjar.

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


Added:
cfe/trunk/test/Parser/skip-function-bodies.h
Modified:
cfe/trunk/include/clang-c/Index.h
cfe/trunk/include/clang/Frontend/ASTUnit.h
cfe/trunk/lib/Frontend/ASTUnit.cpp
cfe/trunk/test/Parser/skip-function-bodies.mm
cfe/trunk/tools/c-index-test/c-index-test.c
cfe/trunk/tools/libclang/CIndex.cpp

Modified: cfe/trunk/include/clang-c/Index.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang-c/Index.h?rev=332578=332577=332578=diff
==
--- cfe/trunk/include/clang-c/Index.h (original)
+++ cfe/trunk/include/clang-c/Index.h Thu May 17 00:31:29 2018
@@ -1321,13 +1321,21 @@ enum CXTranslationUnit_Flags {
*/
   CXTranslationUnit_KeepGoing = 0x200,
 
-  /**
-   * Sets the preprocessor in a mode for parsing a single file only.
-   */
-  CXTranslationUnit_SingleFileParse = 0x400
-};
-
-/**
+  /**
+   * Sets the preprocessor in a mode for parsing a single file only.
+   */
+  CXTranslationUnit_SingleFileParse = 0x400,
+
+  /**
+   * \brief Used in combination with CXTranslationUnit_SkipFunctionBodies to
+   * constrain the skipping of function bodies to the preamble.
+   *
+   * The function bodies of the main file are not skipped.
+   */
+  CXTranslationUnit_LimitSkipFunctionBodiesToPreamble = 0x800,
+};
+
+/**
  * Returns the set of flags that is suitable for parsing a translation
  * unit that is being edited.
  *

Modified: cfe/trunk/include/clang/Frontend/ASTUnit.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/ASTUnit.h?rev=332578=332577=332578=diff
==
--- cfe/trunk/include/clang/Frontend/ASTUnit.h (original)
+++ cfe/trunk/include/clang/Frontend/ASTUnit.h Thu May 17 00:31:29 2018
@@ -78,12 +78,15 @@ class TargetInfo;
 namespace vfs {
 
 class FileSystem;
-
-} // namespace vfs
-
-/// Utility class for loading a ASTContext from an AST file.
-class ASTUnit {
-public:
+
+} // namespace vfs
+
+/// \brief Enumerates the available scopes for skipping function bodies.
+enum class SkipFunctionBodiesScope { None, Preamble, PreambleAndMainFile };
+
+/// Utility class for loading a ASTContext from an AST file.
+class ASTUnit {
+public:
   struct StandaloneFixIt {
 std::pair RemoveRange;
 std::pair InsertFromRange;
@@ -345,12 +348,15 @@ private:
   unsigned CurrentTopLevelHashValue = 0;
   
   /// Bit used by CIndex to mark when a translation unit may be in an
-  /// inconsistent state, and is not safe to free.
-  unsigned UnsafeToFree : 1;
-
-  /// Cache any "global" code-completion results, so that we can avoid
-  /// recomputing them with each completion.
-  void CacheCodeCompletionResults();
+  /// inconsistent state, and is not safe to free.
+  unsigned UnsafeToFree : 1;
+
+  /// \brief Enumerator specifying the scope for skipping function bodies.
+  SkipFunctionBodiesScope SkipFunctionBodies = SkipFunctionBodiesScope::None;
+
+  /// Cache any "global" code-completion results, so that we can avoid
+  /// recomputing them with each completion.
+  void CacheCodeCompletionResults();
   
   /// Clear out and deallocate 
   void ClearCachedCompletionResults();
@@ -360,13 +366,13 @@ private:
   bool Parse(std::shared_ptr PCHContainerOps,
  std::unique_ptr OverrideMainBuffer,
  IntrusiveRefCntPtr VFS);
-
-  std::unique_ptr getMainBufferWithPrecompiledPreamble(
-  std::shared_ptr PCHContainerOps,
-  const CompilerInvocation ,
-  IntrusiveRefCntPtr VFS, bool AllowRebuild = true,
-  unsigned MaxLines = 0);
-  void RealizeTopLevelDeclsFromPreamble();
+
+  std::unique_ptr getMainBufferWithPrecompiledPreamble(
+  std::shared_ptr PCHContainerOps,
+  CompilerInvocation ,
+  IntrusiveRefCntPtr VFS, bool AllowRebuild = true,
+  unsigned MaxLines = 0);
+  void RealizeTopLevelDeclsFromPreamble();
 
   /// Transfers ownership of the objects (like SourceManager) from
   /// \param CI to this ASTUnit.
@@ -798,15 +804,17 @@ public:
   ArrayRef RemappedFiles = None,
   bool RemappedFilesKeepOriginalName = true,
   unsigned PrecompilePreambleAfterNParses = 0,
-  TranslationUnitKind TUKind = TU_Complete,
-  bool 

[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type

[Petr Hosek via Phabricator via cfe-commits]

phosek added inline comments.



Comment at: lib/Index/USRGeneration.cpp:691
+case BuiltinType::ULongAccum:
+  llvm_unreachable("No USR name mangling for fixed point types.");
 case BuiltinType::Float16:

We need some solution for fixed point types.



Comment at: lib/Sema/SemaType.cpp:1391
+if (S.getLangOpts().CPlusPlus) {
+  S.Diag(DS.getTypeSpecTypeLoc(), diag::err_fixed_point_only_allowed_in_c);
+}

Shouldn't there be a break after this line?


https://reviews.llvm.org/D46084



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

Re: r332458 - [AST] Added a helper to extract a user-friendly text of a comment.

[Clement Courbet via cfe-commits]

I should have fixed it in r332576.

On Wed, May 16, 2018 at 11:49 PM, Galina Kistanova via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Also few other builders are affected:
>
> http://lab.llvm.org:8011/builders/clang-x86_64-linux-abi-test
> http://lab.llvm.org:8011/builders/clang-lld-x86_64-2stage
> http://lab.llvm.org:8011/builders/clang-with-lto-ubuntu
>
>
> Thanks
>
> Galina
>
> On Wed, May 16, 2018 at 12:58 PM, Galina Kistanova 
> wrote:
>
>> Hello Ilya,
>>
>> This commit broke build step for couple of our builders:
>>
>> http://lab.llvm.org:8011/builders/clang-with-lto-ubuntu/builds/8541
>> http://lab.llvm.org:8011/builders/clang-with-thin-lto-ubuntu
>>
>> . . .
>> FAILED: 
>> tools/clang/unittests/AST/CMakeFiles/ASTTests.dir/CommentTextTest.cpp.o
>>
>> /usr/bin/c++   -DGTEST_HAS_RTTI=0 -DGTEST_HAS_TR1_TUPLE=0
>> -DGTEST_LANG_CXX11=1 -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS
>> -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/clang/unittests/AST
>> -I/home/buildslave/buildslave1a/clang-with-lto-ubuntu/llvm.src/tools/clang/unittests/AST
>> -I/home/buildslave/buildslave1a/clang-with-lto-ubuntu/llvm.src/tools/clang/include
>> -Itools/clang/include -Iinclude -I/home/buildslave/buildslave1
>> a/clang-with-lto-ubuntu/llvm.src/include -I/home/buildslave/buildslave1
>> a/clang-with-lto-ubuntu/llvm.src/utils/unittest/googletest/include
>> -I/home/buildslave/buildslave1a/clang-with-lto-ubuntu/llvm.
>> src/utils/unittest/googlemock/include -fPIC -fvisibility-inlines-hidden
>> -std=c++11 -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual
>> -Wno-missing-field-initializers -pedantic -Wno-long-long
>> -Wno-maybe-uninitialized -Wdelete-non-virtual-dtor -Wno-comment
>> -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual
>> -fno-strict-aliasing -O3 -DNDEBUG-Wno-variadic-macros -fno-exceptions
>> -fno-rtti -MD -MT tools/clang/unittests/AST/CMak
>> eFiles/ASTTests.dir/CommentTextTest.cpp.o -MF
>> tools/clang/unittests/AST/CMakeFiles/ASTTests.dir/CommentTextTest.cpp.o.d
>> -o tools/clang/unittests/AST/CMakeFiles/ASTTests.dir/CommentTextTest.cpp.o
>> -c /home/buildslave/buildslave1a/clang-with-lto-ubuntu/llvm.src
>> /tools/clang/unittests/AST/CommentTextTest.cpp
>> /home/buildslave/buildslave1a/clang-with-lto-ubuntu/llvm.src
>> /tools/clang/unittests/AST/CommentTextTest.cpp:62:1: error: unterminated
>> raw string
>>  R"cpp(
>>  ^
>> . . .
>>
>> Please have a look?
>>
>> The builder was already red and did not send notifications.
>>
>> Thanks
>>
>> Galina
>>
>>
>>
>> On Wed, May 16, 2018 at 5:30 AM, Ilya Biryukov via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: ibiryukov
>>> Date: Wed May 16 05:30:09 2018
>>> New Revision: 332458
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=332458=rev
>>> Log:
>>> [AST] Added a helper to extract a user-friendly text of a comment.
>>>
>>> Summary:
>>> The helper is used in clangd for documentation shown in code completion
>>> and storing the docs in the symbols. See D45999.
>>>
>>> This patch reuses the code of the Doxygen comment lexer, disabling the
>>> bits that do command and html tag parsing.
>>> The new helper works on all comments, including non-doxygen comments.
>>> However, it does not understand or transform any doxygen directives,
>>> i.e. cannot extract brief text, etc.
>>>
>>> Reviewers: sammccall, hokein, ioeric
>>>
>>> Reviewed By: ioeric
>>>
>>> Subscribers: mgorny, cfe-commits
>>>
>>> Differential Revision: https://reviews.llvm.org/D46000
>>>
>>> Added:
>>> cfe/trunk/unittests/AST/CommentTextTest.cpp
>>> Modified:
>>> cfe/trunk/include/clang/AST/CommentLexer.h
>>> cfe/trunk/include/clang/AST/RawCommentList.h
>>> cfe/trunk/lib/AST/CommentLexer.cpp
>>> cfe/trunk/lib/AST/RawCommentList.cpp
>>> cfe/trunk/unittests/AST/CMakeLists.txt
>>>
>>> Modified: cfe/trunk/include/clang/AST/CommentLexer.h
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/
>>> AST/CommentLexer.h?rev=332458=332457=332458=diff
>>> 
>>> ==
>>> --- cfe/trunk/include/clang/AST/CommentLexer.h (original)
>>> +++ cfe/trunk/include/clang/AST/CommentLexer.h Wed May 16 05:30:09 2018
>>> @@ -281,6 +281,11 @@ private:
>>>/// command, including command marker.
>>>SmallString<16> VerbatimBlockEndCommandName;
>>>
>>> +  /// If true, the commands, html tags, etc will be parsed and reported
>>> as
>>> +  /// separate tokens inside the comment body. If false, the comment
>>> text will
>>> +  /// be parsed into text and newline tokens.
>>> +  bool ParseCommands;
>>> +
>>>/// Given a character reference name (e.g., "lt"), return the
>>> character that
>>>/// it stands for (e.g., "<").
>>>StringRef resolveHTMLNamedCharacterReference(StringRef Name) const;
>>> @@ -315,12 +320,11 @@ private:
>>>/// Eat string matching regexp \code \s*\* \endcode.
>>>void skipLineStartingDecorations();
>>>
>>> -  

r332576 - Fix rL332458: [AST] Added a helper to extract a user-friendly text of a comment.

[Clement Courbet via cfe-commits]

Author: courbet
Date: Wed May 16 23:46:15 2018
New Revision: 332576

URL: http://llvm.org/viewvc/llvm-project?rev=332576=rev
Log:
Fix rL332458: [AST] Added a helper to extract a user-friendly text of a comment.

Older gcc versions do not support raw string literals within macros.

Modified:
cfe/trunk/unittests/AST/CommentTextTest.cpp

Modified: cfe/trunk/unittests/AST/CommentTextTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/CommentTextTest.cpp?rev=332576=332575=332576=diff
==
--- cfe/trunk/unittests/AST/CommentTextTest.cpp (original)
+++ cfe/trunk/unittests/AST/CommentTextTest.cpp Wed May 16 23:46:15 2018
@@ -58,49 +58,54 @@ For example,
this result.
 That's about it.)";
   // Two-slash comments.
-  EXPECT_EQ(ExpectedOutput, formatComment(
+  auto Formatted = formatComment(
 R"cpp(
 // This function does this and that.
 // For example,
 //Runnning it in that case will give you
 //this result.
-// That's about it.)cpp"));
+// That's about it.)cpp");
+  EXPECT_EQ(ExpectedOutput, Formatted);
 
   // Three-slash comments.
-  EXPECT_EQ(ExpectedOutput, formatComment(
+  Formatted = formatComment(
 R"cpp(
 /// This function does this and that.
 /// For example,
 ///Runnning it in that case will give you
 ///this result.
-/// That's about it.)cpp"));
+/// That's about it.)cpp");
+  EXPECT_EQ(ExpectedOutput, Formatted);
 
   // Block comments.
-  EXPECT_EQ(ExpectedOutput, formatComment(
+  Formatted = formatComment(
 R"cpp(
 /* This function does this and that.
  * For example,
  *Runnning it in that case will give you
  *this result.
- * That's about it.*/)cpp"));
+ * That's about it.*/)cpp");
+  EXPECT_EQ(ExpectedOutput, Formatted);
 
   // Doxygen-style block comments.
-  EXPECT_EQ(ExpectedOutput, formatComment(
+  Formatted = formatComment(
 R"cpp(
 /** This function does this and that.
   * For example,
   *Runnning it in that case will give you
   *this result.
-  * That's about it.*/)cpp"));
+  * That's about it.*/)cpp");
+  EXPECT_EQ(ExpectedOutput, Formatted);
 
   // Weird indentation.
-  EXPECT_EQ(ExpectedOutput, formatComment(
+  Formatted = formatComment(
 R"cpp(
// This function does this and that.
   //  For example,
   // Runnning it in that case will give you
 //   this result.
-   // That's about it.)cpp"));
+   // That's about it.)cpp");
+  EXPECT_EQ(ExpectedOutput, Formatted);
   // clang-format on
 }
 
@@ -111,11 +116,12 @@ R"(\brief This is the brief part of the
 \param a something about a.
 @param b something about b.)";
 
-  EXPECT_EQ(ExpectedOutput, formatComment(
+  auto Formatted = formatComment(
 R"cpp(
 /// \brief This is the brief part of the comment.
 /// \param a something about a.
-/// @param b something about b.)cpp"));
+/// @param b something about b.)cpp");
+  EXPECT_EQ(ExpectedOutput, Formatted);
   // clang-format on
 }
 


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

[PATCH] D46998: [XRay][clang+compiler-rt] Make XRay depend on a C++ standard lib

[Dean Michael Berris via Phabricator via cfe-commits]

dberris created this revision.
dberris added reviewers: dblaikie, echristo.

This is related to http://llvm.org/PR32274.

When building with XRay, always depend on a C++ standard library.

We're doing this to automate the linking of the C++ ABI functionality
that the modes use by default. In particular, we depend on some
function-local static initialisation.

The alternative change here is to re-write the modes to only use
libc/pthreads functionality. We're choosing to do this instead as it's
minimally invasive. In the future we can revisit this when we have a
better idea as to why not depending on the C++ ABI functionality is a
better solution.


https://reviews.llvm.org/D46998

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  compiler-rt/test/xray/TestCases/Posix/c-test.cc


Index: compiler-rt/test/xray/TestCases/Posix/c-test.cc
===
--- /dev/null
+++ compiler-rt/test/xray/TestCases/Posix/c-test.cc
@@ -0,0 +1,4 @@
+// RUN: %clang_xray -g -o %t %s
+// REQUIRES: x86_64-target-arch
+// REQUIRES: built-in-llvm-tree
+int main() {}
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -713,7 +713,8 @@
   return !StaticRuntimes.empty() || !NonWholeStaticRuntimes.empty();
 }
 
-bool tools::addXRayRuntime(const ToolChain, const ArgList , 
ArgStringList ) {
+bool tools::addXRayRuntime(const ToolChain , const ArgList ,
+   ArgStringList ) {
   if (Args.hasArg(options::OPT_shared))
 return false;
 
@@ -723,6 +724,11 @@
 for (const auto  : TC.getXRayArgs().modeList())
   CmdArgs.push_back(TC.getCompilerRTArgString(Args, Mode, false));
 CmdArgs.push_back("-no-whole-archive");
+
+// If we're linking a non-C++ application, we'd need to link in the C++
+// runtime.
+if (!Args.hasArg(clang::driver::options::OPT_nostdlibxx))
+  TC.AddCXXStdlibLibArgs(Args, CmdArgs);
 return true;
   }
 


Index: compiler-rt/test/xray/TestCases/Posix/c-test.cc
===
--- /dev/null
+++ compiler-rt/test/xray/TestCases/Posix/c-test.cc
@@ -0,0 +1,4 @@
+// RUN: %clang_xray -g -o %t %s
+// REQUIRES: x86_64-target-arch
+// REQUIRES: built-in-llvm-tree
+int main() {}
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -713,7 +713,8 @@
   return !StaticRuntimes.empty() || !NonWholeStaticRuntimes.empty();
 }
 
-bool tools::addXRayRuntime(const ToolChain, const ArgList , ArgStringList ) {
+bool tools::addXRayRuntime(const ToolChain , const ArgList ,
+   ArgStringList ) {
   if (Args.hasArg(options::OPT_shared))
 return false;
 
@@ -723,6 +724,11 @@
 for (const auto  : TC.getXRayArgs().modeList())
   CmdArgs.push_back(TC.getCompilerRTArgString(Args, Mode, false));
 CmdArgs.push_back("-no-whole-archive");
+
+// If we're linking a non-C++ application, we'd need to link in the C++
+// runtime.
+if (!Args.hasArg(clang::driver::options::OPT_nostdlibxx))
+  TC.AddCXXStdlibLibArgs(Args, CmdArgs);
 return true;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46929: Fix a mangling failure on clang-cl C++17

[Taiju Tsuiki via Phabricator via cfe-commits]

tzik added a comment.

In https://reviews.llvm.org/D46929#1101780, @rnk wrote:

> I searched around, and I noticed that `VTableContext::getMethodVTableIndex` 
> has the same implied contract that the caller must always provide a canonical 
> declaration or things will break. It looks like it has three callers, and 
> they all do this. We should probably sink the canonicalization into this 
> helper as well and clean up the now-superfluous canonicalizations at the call 
> site.


Updated.
As I'm not sure if it's safe to use non-canonicalized CXXMethodDecl for 
EmitVirtualMemPtrThunk and CGCall, I left using the canonical decl.
For other part, non-canonical decl looks working fine to me.


Repository:
  rC Clang

https://reviews.llvm.org/D46929



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