[PATCH] D46835: [ASTImporter] Do not try to remove invisible Decls from DeclContext
a.sidorin added a comment.
Hi Gabor,
I don't feel I'm a right person to review AST-related part so I'm adding other
reviewers.
What I'm worrying about is that there is no test to check if our changes in
removeDecl are correct. Maybe https://reviews.llvm.org/D44100 is a right patch
for this but we should land it first or set dependencies properly.
Regarding ASTImporter, you can find my comments inline.
Comment at: lib/AST/DeclBase.cpp:1386
+// Do not try to remove the declaration if that is invisible to qualified
+// lookup. E.g. template sepcializations are skipped.
+if (shouldBeHidden(ND)) return;
specializations
Comment at: unittests/AST/ASTImporterTest.cpp:1770
+TEST(ImportExpr, ImportClassTemplatePartialSpecialization) {
+ MatchVerifier Verifier;
These tests seem to be for ImportDecl, not for ImportExpr.
Comment at: unittests/AST/ASTImporterTest.cpp:1803
+
+ testImport(Code, Lang_CXX11, "", Lang_CXX11, Verifier, namespaceDecl());
+}
Check for namespaceDecl() looks too weak because import of NamespaceDecl
succeeds even if import of its nested decls fails. Same below.
Comment at: unittests/AST/ASTImporterTest.cpp:1827
+
+TEST_P(ASTImporterTestBase, DISABLED_ImportOfRecordWithDifferentFriends) {
+ Decl *ToR1;
For this change, we should create a separate patch.
Repository:
rC Clang
https://reviews.llvm.org/D46835
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46000: [AST] Added a helper to extract a user-friendly text of a comment.
ilya-biryukov updated this revision to Diff 146757.
ilya-biryukov added a comment.
- Removed the overload that accepts ASTContext
Repository:
rC Clang
https://reviews.llvm.org/D46000
Files:
include/clang/AST/CommentLexer.h
include/clang/AST/RawCommentList.h
lib/AST/CommentLexer.cpp
lib/AST/RawCommentList.cpp
unittests/AST/CMakeLists.txt
unittests/AST/CommentTextTest.cpp
Index: unittests/AST/CommentTextTest.cpp
===
--- /dev/null
+++ unittests/AST/CommentTextTest.cpp
@@ -0,0 +1,122 @@
+//===- unittest/AST/CommentTextTest.cpp - Comment text extraction test ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// Tests for user-friendly output formatting of comments, i.e.
+// RawComment::getFormattedText().
+//
+//===--===//
+
+#include "clang/AST/RawCommentList.h"
+#include "clang/Basic/CommentOptions.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/VirtualFileSystem.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include
+
+namespace clang {
+
+class CommentTextTest : public ::testing::Test {
+protected:
+ std::string formatComment(llvm::StringRef CommentText) {
+SourceManagerForFile FileSourceMgr("comment-test.cpp", CommentText);
+SourceManager& SourceMgr = FileSourceMgr.get();
+
+auto CommentStartOffset = CommentText.find("/");
+assert(CommentStartOffset != llvm::StringRef::npos);
+FileID File = SourceMgr.getMainFileID();
+
+SourceRange CommentRange(
+SourceMgr.getLocForStartOfFile(File).getLocWithOffset(
+CommentStartOffset),
+SourceMgr.getLocForEndOfFile(File));
+CommentOptions EmptyOpts;
+// FIXME: technically, merged that we set here is incorrect, but that
+// shouldn't matter.
+RawComment Comment(SourceMgr, CommentRange, EmptyOpts, /*Merged=*/true);
+DiagnosticsEngine Diags(new DiagnosticIDs, new DiagnosticOptions);
+return Comment.getFormattedText(SourceMgr, Diags);
+ }
+};
+
+TEST_F(CommentTextTest, FormattedText) {
+ // clang-format off
+ auto ExpectedOutput =
+R"(This function does this and that.
+For example,
+ Runnning it in that case will give you
+ this result.
+That's about it.)";
+ // Two-slash comments.
+ EXPECT_EQ(ExpectedOutput, 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"));
+
+ // Three-slash comments.
+ EXPECT_EQ(ExpectedOutput, 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"));
+
+ // Block comments.
+ EXPECT_EQ(ExpectedOutput, 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"));
+
+ // Doxygen-style block comments.
+ EXPECT_EQ(ExpectedOutput, 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"));
+
+ // Weird indentation.
+ EXPECT_EQ(ExpectedOutput, 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"));
+ // clang-format on
+}
+
+TEST_F(CommentTextTest, KeepsDoxygenControlSeqs) {
+ // clang-format off
+ auto ExpectedOutput =
+R"(\brief This is the brief part of the comment.
+\param a something about a.
+@param b something about b.)";
+
+ EXPECT_EQ(ExpectedOutput, formatComment(
+R"cpp(
+/// \brief This is the brief part of the comment.
+/// \param a something about a.
+/// @param b something about b.)cpp"));
+ // clang-format on
+}
+
+} // namespace clang
Index: unittests/AST/CMakeLists.txt
===
--- unittests/AST/CMakeLists.txt
+++ unittests/AST/CMakeLists.txt
@@ -9,6 +9,7 @@
ASTVectorTest.cpp
CommentLexer.cpp
CommentParser.cpp
+ CommentTextTest.cpp
DataCollectionTest.cpp
DeclPrinterTest.cpp
DeclTest.cpp
Index: lib/AST/RawCommentList.cpp
===
--- lib/AST/RawCommentList.cpp
+++ lib/AST/RawCommentList.cpp
@@ -335,3 +335,94 @@
BeforeThanCompare(SourceMgr));
std::swap(Comments, MergedComments);
}
+
+std::str
[PATCH] D46000: [AST] Added a helper to extract a user-friendly text of a comment.
ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lgtm Comment at: include/clang/AST/RawCommentList.h:138 + /// the overload with ASTContext in the rest of the code. + std::string getFormattedText(const SourceManager &SourceMgr, + DiagnosticsEngine &Diags) const; ilya-biryukov wrote: > ioeric wrote: > > I think we can get rid of the interface that takes `ASTContext`? If > > `SourceManager` and `Diags` are sufficient, I don't see why we would want > > another interface for ASTContext. > Two reasons that come to mind: it's simpler to use and follows the API of > `getBriefText`. If not for mocking the tests, I would totally only keep the > `ASTContext`-based one, since it does not really make any sense to create > `RawComment` without `ASTContext` for any reason other than testing. As discussed offline, two interfaces that do the same thing are a bit confusing. I would still suggest favoring minimal API and test-ability over simplified usage - two parameters aren't that better than one after all :) Comment at: lib/AST/RawCommentList.cpp:352 + // comments::Lexer. Therefore, we just use default-constructed options. + CommentOptions DefOpts; + comments::CommandTraits EmptyTraits(Allocator, DefOpts); ilya-biryukov wrote: > ioeric wrote: > > I'm not quite sure about this. Could we just require a `CommandTraits` in > > the interface? And only make this assumption in tests? > I think we shouldn't add this to params, the whole point of this function is > to do parsing that ignores the commands and the `CommandTraits`. The fact > that lexer still needs them is because we haven't extracted a simpler > interface from `Lexer` that doesn't rely on unused params, i.e. > `CommandTraits` and `Allocator`. Makes sense. Repository: rC Clang https://reviews.llvm.org/D46000 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46353: [ASTImporter] Extend lookup logic in class templates
martong updated this revision to Diff 146765.
martong added a comment.
- Add FIXME
Repository:
rC Clang
https://reviews.llvm.org/D46353
Files:
lib/AST/ASTImporter.cpp
unittests/AST/ASTImporterTest.cpp
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1431,6 +1431,39 @@
MatchVerifier{}.match(To->getTranslationUnitDecl(), Pattern));
}
+TEST_P(ASTImporterTestBase, ImportDefinitionOfClassTemplateAfterFwdDecl) {
+ {
+Decl *FromTU = getTuDecl(
+R"(
+template
+struct B;
+)",
+Lang_CXX, "input0.cc");
+auto *FromD = FirstDeclMatcher().match(
+FromTU, classTemplateDecl(hasName("B")));
+
+Import(FromD, Lang_CXX);
+ }
+
+ {
+Decl *FromTU = getTuDecl(
+R"(
+template
+struct B {
+ void f();
+};
+)",
+Lang_CXX, "input1.cc");
+FunctionDecl *FromD = FirstDeclMatcher().match(
+FromTU, functionDecl(hasName("f")));
+Import(FromD, Lang_CXX);
+auto *FromCTD = FirstDeclMatcher().match(
+FromTU, classTemplateDecl(hasName("B")));
+auto *ToCTD = cast(Import(FromCTD, Lang_CXX));
+EXPECT_TRUE(ToCTD->isThisDeclarationADefinition());
+ }
+}
+
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
@@ -4077,8 +4077,14 @@
if (auto *FoundTemplate = dyn_cast(Found)) {
if (IsStructuralMatch(D, FoundTemplate)) {
// The class templates structurally match; call it the same template.
- // FIXME: We may be filling in a forward declaration here. Handle
- // this case!
+
+ // We found a forward declaration but the class to be imported has a
+ // definition.
+ // FIXME Add this forward declaration to the redeclaration chain.
+ if (D->isThisDeclarationADefinition() &&
+ !FoundTemplate->isThisDeclarationADefinition())
+continue;
+
Importer.Imported(D->getTemplatedDecl(),
FoundTemplate->getTemplatedDecl());
return Importer.Imported(D, FoundTemplate);
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1431,6 +1431,39 @@
MatchVerifier{}.match(To->getTranslationUnitDecl(), Pattern));
}
+TEST_P(ASTImporterTestBase, ImportDefinitionOfClassTemplateAfterFwdDecl) {
+ {
+Decl *FromTU = getTuDecl(
+R"(
+template
+struct B;
+)",
+Lang_CXX, "input0.cc");
+auto *FromD = FirstDeclMatcher().match(
+FromTU, classTemplateDecl(hasName("B")));
+
+Import(FromD, Lang_CXX);
+ }
+
+ {
+Decl *FromTU = getTuDecl(
+R"(
+template
+struct B {
+ void f();
+};
+)",
+Lang_CXX, "input1.cc");
+FunctionDecl *FromD = FirstDeclMatcher().match(
+FromTU, functionDecl(hasName("f")));
+Import(FromD, Lang_CXX);
+auto *FromCTD = FirstDeclMatcher().match(
+FromTU, classTemplateDecl(hasName("B")));
+auto *ToCTD = cast(Import(FromCTD, Lang_CXX));
+EXPECT_TRUE(ToCTD->isThisDeclarationADefinition());
+ }
+}
+
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
@@ -4077,8 +4077,14 @@
if (auto *FoundTemplate = dyn_cast(Found)) {
if (IsStructuralMatch(D, FoundTemplate)) {
// The class templates structurally match; call it the same template.
- // FIXME: We may be filling in a forward declaration here. Handle
- // this case!
+
+ // We found a forward declaration but the class to be imported has a
+ // definition.
+ // FIXME Add this forward declaration to the redeclaration chain.
+ if (D->isThisDeclarationADefinition() &&
+ !FoundTemplate->isThisDeclarationADefinition())
+continue;
+
Importer.Imported(D->getTemplatedDecl(),
FoundTemplate->getTemplatedDecl());
return Importer.Imported(D, FoundTemplate);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46353: [ASTImporter] Extend lookup logic in class templates
martong added a comment. Hi Aleksei, Added the FIXME, can you help me with committing this? Repository: rC Clang https://reviews.llvm.org/D46353 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46751: [clangd] Filter out private proto symbols in SymbolCollector.
ioeric updated this revision to Diff 146770.
ioeric added a comment.
- Add heuristic to reduce false position on identifying proto headers
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D46751
Files:
clangd/ClangdLSPServer.cpp
clangd/SourceCode.cpp
clangd/SourceCode.h
clangd/index/SymbolCollector.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -697,6 +697,39 @@
AllOf(QName("pörk"), DeclRange(Header.range();
}
+TEST_F(SymbolCollectorTest, FilterPrivateProtoSymbols) {
+ TestHeaderName = testPath("x.proto.h");
+ const std::string Header =
+ R"(// Generated by the protocol buffer compiler. DO NOT EDIT!
+ namespace nx {
+ class Top_Level {};
+ class TopLevel {};
+ enum Kind {
+ KIND_OK,
+ Kind_Not_Ok,
+ };
+ })";
+ runSymbolCollector(Header, /*Main=*/"");
+ EXPECT_THAT(Symbols, UnorderedElementsAre(QName("nx"), QName("nx::TopLevel"),
+QName("nx::Kind"),
+QName("nx::KIND_OK")));
+}
+
+TEST_F(SymbolCollectorTest, DoubleCheckProtoHeaderComment) {
+ TestHeaderName = testPath("x.proto.h");
+ const std::string Header = R"(
+ namespace nx {
+class Top_Level {};
+enum Kind {
+ Kind_Fine
+};
+ }
+ )";
+ runSymbolCollector(Header, /*Main=*/"");
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(QName("nx"), QName("nx::Top_Level"),
+ QName("nx::Kind"), QName("nx::Kind_Fine")));
+}
} // namespace
} // namespace clangd
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -90,6 +90,39 @@
return llvm::None;
}
+// All proto generated headers should start with this line.
+static const char *PROTO_HEADER_COMMENT =
+"// Generated by the protocol buffer compiler. DO NOT EDIT!";
+
+// Checks whether the decl is a private symbol in a header generated by
+// protobuf compiler.
+// To identify whether a proto header is actually generated by proto compiler,
+// we check whether it starts with PROTO_HEADER_COMMENT.
+bool isPrivateProtoSymbol(const NamedDecl &ND) {
+ const auto &SM = ND.getASTContext().getSourceManager();
+ auto Loc = findNameLoc(&ND);
+ auto FileName = SM.getFilename(Loc);
+ if (!FileName.endswith(".proto.h") && !FileName.endswith(".pb.h"))
+return false;
+ auto FID = SM.getFileID(Loc);
+ // Double check that this is an actual protobuf header.
+ if (!SM.getBufferData(FID).startswith(PROTO_HEADER_COMMENT))
+return false;
+
+ auto Name = ND.getName();
+ if (!Name.contains('_'))
+return false;
+ // Nested proto entities (e.g. Message::Nested) have top-level decls
+ // that shouldn't be used (Message_Nested). Ignore them completely.
+ // The nested entities are dangling type aliases, we may want to reconsider
+ // including them in the future.
+ // For enum constants, SOME_ENUM_CONSTANT is not private and should be
+ // indexed. Outer_INNER is private. This heuristic relies on naming style, it
+ // will include OUTER_INNER and exclude some_enum_constant.
+ return (ND.getKind() != Decl::EnumConstant) ||
+ std::any_of(Name.begin(), Name.end(), islower);
+}
+
bool shouldFilterDecl(const NamedDecl *ND, ASTContext *ASTCtx,
const SymbolCollector::Options &Opts) {
using namespace clang::ast_matchers;
@@ -130,6 +163,9 @@
.empty())
return true;
+ // Avoid indexing internal symbols in protobuf generated headers.
+ if (isPrivateProtoSymbol(*ND))
+return true;
return false;
}
Index: clangd/SourceCode.h
===
--- clangd/SourceCode.h
+++ clangd/SourceCode.h
@@ -15,6 +15,7 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SOURCECODE_H
#include "Protocol.h"
#include "clang/Basic/SourceLocation.h"
+#include "clang/Tooling/Core/Replacement.h"
namespace clang {
class SourceManager;
@@ -55,6 +56,11 @@
std::pair
splitQualifiedName(llvm::StringRef QName);
+TextEdit replacementToEdit(StringRef Code, const tooling::Replacement &R);
+
+std::vector replacementsToEdits(StringRef Code,
+ const tooling::Replacements &Repls);
+
} // namespace clangd
} // namespace clang
#endif
Index: clangd/SourceCode.cpp
===
--- clangd/SourceCode.cpp
+++ clangd/SourceCode.cpp
@@ -166,5 +166,20 @@
return {QName.substr(0, Pos + 2), QName.substr(Pos + 2)};
}
+TextEdit replacementToEdit(StringRef Code, const tooling::Replacement &R) {
+ Range Replacem
[PATCH] D46751: [clangd] Filter out private proto symbols in SymbolCollector.
ioeric added a comment. > I think this is good if that's true that the comment is always there. I think > it would be OK for this to be enabled by default, with a general option to > turn heuristics off. Not sure what to call it... > -use-symbol-filtering-heuristics :) @malaperle Having an option for filtering heuristics seems a bit confusing. We have other filters in the symbol collector that we think could improve user experience, and we don't provide options for those. Similarly, for proto headers, I think we could also get away without such an option if we strike for low/no false positive (e.g. correctly identify proto headers). If folks run into problems with the filter, we would like to understand the use cases and improve the filters. In general, we think the proto filter, when it works, would improve user experience. >> I'm not a big fan of parsing file comment in proto. It seems a bit >> cumbersome and we might not be able (or too expensive) to do so for >> completion results from sema (if we do want to filter at completion time). > > Why do you feel it's cubersome? Getting the first line of the file from > SourceManager and looking at it seems easy. > I certainly don't see why this should be expensive if we do it at the right > time (perhaps it means doing that when building the preamble and stashing the > results alongside it, but that's also easy in our current setup). @ilya-biryukov You are right, getting a working solution seems easy enough. I was more concerned about finding a design that doesn't intrude completion workflow with library specific logic. But this makes more sense now that we are not doing the filtering on code completion workflow. Comment at: clangd/index/SymbolCollector.cpp:156 + if (isPrivateProtoSymbol(ND, ASTCtx->getSourceManager())) +return true; + ilya-biryukov wrote: > ioeric wrote: > > ilya-biryukov wrote: > > > We should also run the same code to filter our those decls coming from > > > sema completions. > > > Otherwise, they might still pop up in completion results if internal > > > proto symbols were deserialized from preamble by clang before running the > > > code completion. > > Just want to clarify before going further. IIUC, in *index-based* > > completion, the preamble could still contain symbols from headers such that > > sema completion could still give you symbols from headers. > > > > If we do need to build the filter into code completion, we would need more > > careful design as code completion code path is more latency sensitive. > For reference. As discussed outside this thread, we might have decls from > headers in the sema completion items. > It does not seem too hard to add filtering for those as well: essentially, we > just need to call the same function at code completion time. After further discussion, we agreed that filtering on code completion results may not be the right approach. For example, it would break assumptions in code completion workflow e.g. limit of completion results from index. Alternatively, we could push filtering to the symbol source level. Currently, we have two sources of symbols: sema and index, and both of them can gather symbols from headers, which are then merged in code completion. With this setup, we would need to apply filters on both sources if we want to do any filtering on header symbols. One solution (for index-based completion) is to make sema only collect symbols in main file (just for code completion) and make indexer index headers (current behavior), where we would only need to filter on index. This doesn't address problem for sema-only code completion, but we think it's not a priority to strike for feature parity between sema-based completion and index-based completion, which we don't really have at this point. So to proceed: 1) I'll go ahead with the current approach (filter index only) with a stricter check for proto headers. 2) Make sema only collect symbols in main files. 3) Potentially also apply the filter on sema completion results. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46751 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46863: [X86] Use __builtin_convertvector to implement some of the packed integer to packed flow conversion intrinsics.
RKSimon added reviewers: efriedma, hfinkel. RKSimon added a comment. I'm all for keeping the scalar/vector behaviour the same but I'm concerned about constant folding not taking into account runtime rounding mode: e,.g. SelectionDAG::getNode - we don't check the return status of convertFromAPInt from *INT_TO_FP - but then again the FP_TO_*INT constant folds only bail on invalid conversions. I haven't checked what other passes are doing. Repository: rC Clang https://reviews.llvm.org/D46863 ___ 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
ilya-biryukov added a comment.
Maybe move the tests back to this revision?
There are tests for code completion that don't depend on libindex or libclang
and use clang -cc1 directly, i.e. `tools/clang/test/CodeComplete`. This would
require adding a flag to clang and patching up `PrintingCodeCompleConsumer` to
print the fix-its, but that should be easy. Moreover, it's useful to have the
option to show those completions in clang anyway.
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5715
"member reference type %0 is %select{a|not a}1 pointer; did you mean to use
'%select{->|.}1'?">;
+def note_typecheck_member_reference_full_suggestion : Note<
+ "member reference type %0 may %select{|not}1 be pointer; did you mean to use
'%select{->|.}1'?">;
This is not used anymore, right? Maybe remove it?
Comment at: include/clang/Sema/CodeCompleteConsumer.h:807
+ /// vec_ptr.^ // completion returns an item 'push_back', replacing '.'
+ /// with '->' vec_ptr->^ // completion returns an item 'release',
+ /// replacing '->' with '.'
NIT: reflow on this comment broke formatting, maybe keep it unindented
instead? I.e., this should give better formatting:
```
/// std::unique_ptr> vec_ptr;
/// vec_ptr.^
/// vec_ptr->^
///
/// In 'vec_ptr.^', one of completion items is 'push_back', it requires
replacing '.' with '->'.
/// In 'vec_ptr->^', one of completion items is 'release', it requires
replacing '.' with '->'.
```
Comment at: lib/Sema/SemaCodeComplete.cpp:3954
+ RecordDecl *RD,
+ std::vector &&FixIts)
{
// Indicate that we are performing a member access, and the cv-qualifiers
Maybe pass a single fix-it here too with a more descriptive name?
It would make the code more readable.
Comment at: lib/Sema/SemaCodeComplete.cpp:4022
+
+ auto DoCompletion = [&](Expr *Base, bool IsArrow, std::vector
&&FixIts) -> bool {
+if (!Base)
Maybe pass a single fix-it with more descriptive name, e.g.
`Optional AccessOpFixIt`?
Comment at: lib/Sema/SemaCodeComplete.cpp:4105
+std::vector FixIts {FixItHint::CreateReplacement(OpLoc, Corr)};
+//FIXME: Add assert to check FixIt range requirements.
+
This FIXME should probably live in `CodeCompletionResult` constructor or
`ResultBuilder::AddResult`. It's pretty obvious that this specific code does
the right thing, but it may be non-obvious about more generic code that stores
a `vector`
Comment at: test/SemaCXX/member-expr.cpp:193
+Cl0* c;
+return c.a; // expected-error {{member reference type 'PR15045::Cl0 *' is
a pointer; did you mean to use '->'?}}
+ }
Is this still needed after we removed the diagnostics code?
Comment at: tools/libclang/CIndexCodeCompletion.cpp:309
+ /// before that result for the corresponding completion item.
+ std::vector> FixItsVector;
};
yvvan wrote:
> ilya-biryukov wrote:
> > yvvan wrote:
> > > ilya-biryukov wrote:
> > > > Storing `vector>` here makes looks like a hack. Even though it
> > > > might seem more tricky, I suggest storing an opaque pointer to
> > > > `vector` in each `CXCompletionResult`. Managing the
> > > > lifetime of vectors in the `AllocatedCXCodeCompleteResults` seems
> > > > totally fine, but there should be a way to get to the fixits in a
> > > > similar way we can get to the completion string.
> > > > More concretely, I suggest the following API:
> > > > ```
> > > > // === Index.h
> > > > typedef void* CXCompletionFixIts;
> > > > typedef struct {
> > > >// ...
> > > >CXCompletionFixIts FixIts;
> > > > };
> > > >
> > > > // Get the number of fix-its.
> > > > unsigned clang_getCompletionNumFixIts(CXCompletionResult *Result);
> > > > // ... Similar to getDiagnosticFixIt
> > > > CXString clang_getCompletionFixIt((CXCompletionResult *Result, unsigned
> > > > fixit_index, CXSourceRange *replacement_range);
> > > >
> > > >
> > > >
> > > > // === Impl.cpp
> > > > struct AllocatedCXCodeCompleteResults : public CXCodeCompleteResults {
> > > > // .
> > > > // Pool for allocating non-empty fixit vectors in the
> > > > CXCompletionResult.
> > > > std::vector> FixItsVector
> > > > };
> > > >
> > > > unsigned clang_getCompletionNumFixIts(CXCompletionResult *Result) {
> > > > auto* FixIts = static_cast*>(Result->FixIts);
> > > > if (!FixIts)
> > > > return 0;
> > > > return FixIts->size();
> > > > }
> > > > ```
> > > unsigned clang_getCompletionNumFixIts(CXCompletionResult *Result);
> > >
> > > Do you mean appending CXCompletionResult struct with the
> > > "CXCompletionFixIts FixIts" member? Doesn't it break binary compatibility
> > > (changing the struct size)?
> > Ah, you're right. If libclang pro
r332335 - Make ast-print-record-decl.c pass on Windows after r332314
Author: hans Date: Tue May 15 03:19:24 2018 New Revision: 332335 URL: http://llvm.org/viewvc/llvm-project?rev=332335&view=rev Log: Make ast-print-record-decl.c pass on Windows after r332314 It was failing because on Windows, -ast-print prints __single_inheritance(1) before T1. Adding a triple is a stop-gap fix until it can be fixed properly. Modified: cfe/trunk/test/Misc/ast-print-record-decl.c Modified: cfe/trunk/test/Misc/ast-print-record-decl.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/ast-print-record-decl.c?rev=332335&r1=332334&r2=332335&view=diff == --- cfe/trunk/test/Misc/ast-print-record-decl.c (original) +++ cfe/trunk/test/Misc/ast-print-record-decl.c Tue May 15 03:19:24 2018 @@ -54,7 +54,7 @@ // RUN:-DKW=struct -DBASES=' : B' -o - -xc++ %s \ // RUN: | FileCheck --check-prefixes=CHECK,LLVM %s // -// RUN: %clang_cc1 -verify -ast-print -DKW=struct -DBASES=' : B' -xc++ %s \ +// RUN: %clang_cc1 -verify -triple x86_64-linux -ast-print -DKW=struct -DBASES=' : B' -xc++ %s \ // RUN: > %t.cpp // RUN: FileCheck --check-prefixes=CHECK,PRINT,PRINT-CXX -DKW=struct \ // RUN: -DBASES=' : B' %s --input-file %t.cpp @@ -67,7 +67,7 @@ // RUN: %clang -target x86_64-linux -Xclang -verify -S -emit-llvm -o - %t.cpp \ // RUN: | FileCheck --check-prefixes=CHECK,LLVM %s // -// RUN: %clang_cc1 -verify -ast-print %t.cpp \ +// RUN: %clang_cc1 -verify -triple x86_64-linux -ast-print %t.cpp \ // RUN: | FileCheck --check-prefixes=CHECK,PRINT,PRINT-CXX -DKW=struct \ // RUN: -DBASES=' : B' %s ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r332314 - [AST] Fix printing tag decl groups in decl contexts
This broke the ast-print-record-decl.c test on Windows, see for
example
http://lab.llvm.org:8011/builders/clang-with-thin-lto-windows/builds/9066
One way to reproduce the failure on Linux is to pass a Windows triple
to this ast-print command:
--- a/test/Misc/ast-print-record-decl.c
+++ b/test/Misc/ast-print-record-decl.c
@@ -54,7 +54,7 @@
// RUN:-DKW=struct -DBASES=' : B' -o - -xc++ %s \
// RUN: | FileCheck --check-prefixes=CHECK,LLVM %s
//
-// RUN: %clang_cc1 -verify -ast-print -DKW=struct -DBASES=' : B' -xc++ %s \
+// RUN: %clang_cc1 -verify -triple i686-pc-win32 -ast-print
-DKW=struct -DBASES=' : B' -xc++ %s \
// RUN: > %t.cpp
// RUN: FileCheck --check-prefixes=CHECK,PRINT,PRINT-CXX -DKW=struct \
// RUN: -DBASES=' : B' %s --input-file %t.cpp
What's happening is that on Windows, "__single_inheritance(1)" is
printed before T1. But if I change the test to allow that in the
output, it still doesn't pass as Clang doesn't seem able to parse it.
I've worked around the problem by adding a Linux triple here in
r332335, but someone should probably look into it properly.
Thanks,
Hans
On Tue, May 15, 2018 at 2:44 AM, Joel E. Denny via cfe-commits
wrote:
> Author: jdenny
> Date: Mon May 14 17:44:14 2018
> New Revision: 332314
>
> URL: http://llvm.org/viewvc/llvm-project?rev=332314&view=rev
> Log:
> [AST] Fix printing tag decl groups in decl contexts
>
> For example, given:
>
> struct T1 {
> struct T2 *p0;
> };
>
> -ast-print produced:
>
> struct T1 {
> struct T2;
> struct T2 *p0;
> };
>
> Compiling that produces a warning that the first struct T2 declaration
> does not declare anything.
>
> Details:
>
> A tag decl group is one or more decls that share a type specifier that
> is a tag decl (that is, a struct/union/class/enum decl). Within
> functions, the parser builds such a tag decl group as part of a
> DeclStmt. However, in decl contexts, such as file scope or a member
> list, the parser does not group together the members of a tag decl
> group. Previously, detection of tag decl groups during printing was
> implemented but only if the tag decl was unnamed. Otherwise, as in
> the above example, the members of the group did not print together and
> so sometimes introduced warnings.
>
> This patch extends detection of tag decl groups in decl contexts to
> any tag decl that is recorded in the AST as not free-standing.
>
> Reviewed by: rsmith
>
> Differential Revision: https://reviews.llvm.org/D45465
>
> Modified:
> cfe/trunk/lib/AST/DeclPrinter.cpp
> cfe/trunk/test/Misc/ast-print-enum-decl.c
> cfe/trunk/test/Misc/ast-print-record-decl.c
> cfe/trunk/test/Sema/ast-print.c
> cfe/trunk/test/SemaCXX/ast-print.cpp
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46867: [ASTImporter] Add unit tests for structural equivalence
martong created this revision.
martong added reviewers: a.sidorin, xazax.hun, szepet.
Herald added subscribers: cfe-commits, dkrupp, rnkovacs, mgorny.
This patch add new tests for structural equivalence. For that a new common
header is created which holds the test related language specific types and
functions.
Repository:
rC Clang
https://reviews.llvm.org/D46867
Files:
unittests/AST/ASTImporterTest.cpp
unittests/AST/CMakeLists.txt
unittests/AST/Language.h
unittests/AST/MatchVerifier.h
unittests/AST/StructuralEquivalenceTest.cpp
Index: unittests/AST/StructuralEquivalenceTest.cpp
===
--- /dev/null
+++ unittests/AST/StructuralEquivalenceTest.cpp
@@ -0,0 +1,211 @@
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTImporter.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/AST/ASTStructuralEquivalence.h"
+#include "clang/Frontend/ASTUnit.h"
+#include "clang/Tooling/Tooling.h"
+
+#include "Language.h"
+#include "DeclMatcher.h"
+
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace ast_matchers {
+
+struct StructuralEquivalenceTest : ::testing::Test {
+ std::unique_ptr AST0, AST1;
+ std::string Code0, Code1; // Buffers for SourceManager
+
+ // Get a pair of Decl pointers to the synthetised declarations from the given
+ // code snipets. By default we search for the unique Decl with name 'foo' in
+ // both snippets.
+ std::tuple
+ makeNamedDecls(const std::string &SrcCode0, const std::string &SrcCode1,
+ Language Lang, const char *const Identifier = "foo") {
+
+this->Code0 = SrcCode0;
+this->Code1 = SrcCode1;
+ArgVector Args = getBasicRunOptionsForLanguage(Lang);
+
+const char *const InputFileName = "input.cc";
+
+AST0 = tooling::buildASTFromCodeWithArgs(Code0, Args, InputFileName);
+AST1 = tooling::buildASTFromCodeWithArgs(Code1, Args, InputFileName);
+
+ASTContext &Ctx0 = AST0->getASTContext(), &Ctx1 = AST1->getASTContext();
+
+auto getDecl = [](ASTContext &Ctx, const std::string &Name) -> NamedDecl * {
+ IdentifierInfo *ImportedII = &Ctx.Idents.get(Name);
+ assert(ImportedII && "Declaration with the identifier "
+ "should be specified in test!");
+ DeclarationName ImportDeclName(ImportedII);
+ SmallVector FoundDecls;
+ Ctx.getTranslationUnitDecl()->localUncachedLookup(ImportDeclName,
+FoundDecls);
+
+ // We should find one Decl but one only
+ assert(FoundDecls.size() > 0);
+ assert(FoundDecls.size() < 2);
+
+ return FoundDecls[0];
+};
+
+NamedDecl *d0 = getDecl(Ctx0, Identifier);
+NamedDecl *d1 = getDecl(Ctx1, Identifier);
+assert(d0);
+assert(d1);
+return std::make_tuple(d0, d1);
+ }
+
+ bool testStructuralMatch(NamedDecl *d0, NamedDecl *d1) {
+llvm::DenseSet> NonEquivalentDecls;
+StructuralEquivalenceContext Ctx(d0->getASTContext(), d1->getASTContext(),
+ NonEquivalentDecls, false, false);
+return Ctx.IsStructurallyEquivalent(d0, d1);
+ }
+};
+
+using std::get;
+
+TEST_F(StructuralEquivalenceTest, Int) {
+ auto t = makeNamedDecls("int foo;", "int foo;", Lang_CXX);
+ EXPECT_TRUE(testStructuralMatch(get<0>(t), get<1>(t)));
+}
+
+TEST_F(StructuralEquivalenceTest, IntVsSignedInt) {
+ auto t = makeNamedDecls("int foo;", "signed int foo;", Lang_CXX);
+ EXPECT_TRUE(testStructuralMatch(get<0>(t), get<1>(t)));
+}
+
+TEST_F(StructuralEquivalenceTest, Char) {
+ auto t = makeNamedDecls("char foo;", "char foo;", Lang_CXX);
+ EXPECT_TRUE(testStructuralMatch(get<0>(t), get<1>(t)));
+}
+
+// This test is disabled for now.
+// FIXME Whether this is equivalent is dependendant on the target.
+TEST_F(StructuralEquivalenceTest, DISABLED_CharVsSignedChar) {
+ auto t = makeNamedDecls("char foo;", "signed char foo;", Lang_CXX);
+ EXPECT_FALSE(testStructuralMatch(get<0>(t), get<1>(t)));
+}
+
+TEST_F(StructuralEquivalenceTest, ForwardRecordDecl) {
+ auto t = makeNamedDecls("struct foo;", "struct foo;", Lang_CXX);
+ EXPECT_TRUE(testStructuralMatch(get<0>(t), get<1>(t)));
+}
+
+TEST_F(StructuralEquivalenceTest, IntVsSignedIntInStruct) {
+ auto t = makeNamedDecls("struct foo { int x; };",
+ "struct foo { signed int x; };", Lang_CXX);
+ EXPECT_TRUE(testStructuralMatch(get<0>(t), get<1>(t)));
+}
+
+TEST_F(StructuralEquivalenceTest, CharVsSignedCharInStruct) {
+ auto t = makeNamedDecls("struct foo { char x; };",
+ "struct foo { signed char x; };", Lang_CXX);
+ EXPECT_FALSE(testStructuralMatch(get<0>(t), get<1>(t)));
+}
+
+TEST_F(StructuralEquivalenceTest, IntVsSignedIntTemplateSpec) {
+ auto t = makeNamedDecls(
+ "template struct foo; template<> struct foo{};",
+ "template struct foo; template<> struct foo{};",
+ Lang_CXX);
+ ClassTemplateSpecializationDecl *Spec0 =
+ *cast(get<0>(t))->spec_beg
[PATCH] D41537: Optionally add code completion results for arrow instead of dot
yvvan added a comment.
I will add more tests...
Comment at: test/SemaCXX/member-expr.cpp:193
+Cl0* c;
+return c.a; // expected-error {{member reference type 'PR15045::Cl0 *' is
a pointer; did you mean to use '->'?}}
+ }
ilya-biryukov wrote:
> Is this still needed after we removed the diagnostics code?
Yes, even if it does not fail it is not related to that change anymore
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] D46485: Add python tool to dump and construct header maps
jkorous added a comment.
Hi Bruno, I just noticed couple of implementation details.
Comment at: utils/hmaptool/hmaptool:55
+# The number of buckets must be a power of two.
+if num_buckets == 0 or (num_buckets & num_buckets - 1) != 0:
+raise SystemExit("error: %s: invalid number of buckets" % (
Wouldn't it be simpler to use modulo 2?
```
if num_buckets % 2 == 0
```
Comment at: utils/hmaptool/hmaptool:153
+def next_power_of_two(value):
+for i in range(32):
+power = 1 << i
This is probably not critically important but maybe we could use math functions
instead of iteration here:
```
from math import *
return ceil( log( value + 1, 2) )
```
...or if we want to support values <1:
```
if value <= 0
raise ArgumentError
if value < 1
return 0
return ceil( log( value, 2) ) + 1
```
Comment at: utils/hmaptool/hmaptool:234
+
+if len(args) != 2:
+parser.error("invalid number of arguments")
Aren't we expecting just single argument () here?
Repository:
rC Clang
https://reviews.llvm.org/D46485
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46439: Fix incorrect packed aligned structure layout
chill added a comment. Ping? https://reviews.llvm.org/D46439 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46603: [Support] TimerGroup changes
lebedev.ri added a comment. Ping. Any thoughts on these Timer changes? Repository: rL LLVM https://reviews.llvm.org/D46603 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r332338 - [ASTImporter] Extend lookup logic in class templates
Author: a.sidorin
Date: Tue May 15 04:09:07 2018
New Revision: 332338
URL: http://llvm.org/viewvc/llvm-project?rev=332338&view=rev
Log:
[ASTImporter] Extend lookup logic in class templates
During import of a class template, lookup may find a forward
declaration and structural match falsely reports equivalency
between a forward decl and a definition. The result is that
some definitions are not imported if we had imported a forward
decl previously. This patch gives a fix.
Patch by Gabor Marton!
Differential Revision: https://reviews.llvm.org/D46353
Modified:
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/unittests/AST/ASTImporterTest.cpp
Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=332338&r1=332337&r2=332338&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Tue May 15 04:09:07 2018
@@ -4108,8 +4108,14 @@ Decl *ASTNodeImporter::VisitClassTemplat
if (auto *FoundTemplate = dyn_cast(Found)) {
if (IsStructuralMatch(D, FoundTemplate)) {
// The class templates structurally match; call it the same template.
- // FIXME: We may be filling in a forward declaration here. Handle
- // this case!
+
+ // We found a forward declaration but the class to be imported has a
+ // definition.
+ // FIXME Add this forward declaration to the redeclaration chain.
+ if (D->isThisDeclarationADefinition() &&
+ !FoundTemplate->isThisDeclarationADefinition())
+continue;
+
Importer.Imported(D->getTemplatedDecl(),
FoundTemplate->getTemplatedDecl());
return Importer.Imported(D, FoundTemplate);
Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterTest.cpp?rev=332338&r1=332337&r2=332338&view=diff
==
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp (original)
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp Tue May 15 04:09:07 2018
@@ -1431,6 +1431,39 @@ TEST_P(ASTImporterTestBase,
MatchVerifier{}.match(To->getTranslationUnitDecl(), Pattern));
}
+TEST_P(ASTImporterTestBase, ImportDefinitionOfClassTemplateAfterFwdDecl) {
+ {
+Decl *FromTU = getTuDecl(
+R"(
+template
+struct B;
+)",
+Lang_CXX, "input0.cc");
+auto *FromD = FirstDeclMatcher().match(
+FromTU, classTemplateDecl(hasName("B")));
+
+Import(FromD, Lang_CXX);
+ }
+
+ {
+Decl *FromTU = getTuDecl(
+R"(
+template
+struct B {
+ void f();
+};
+)",
+Lang_CXX, "input1.cc");
+FunctionDecl *FromD = FirstDeclMatcher().match(
+FromTU, functionDecl(hasName("f")));
+Import(FromD, Lang_CXX);
+auto *FromCTD = FirstDeclMatcher().match(
+FromTU, classTemplateDecl(hasName("B")));
+auto *ToCTD = cast(Import(FromCTD, Lang_CXX));
+EXPECT_TRUE(ToCTD->isThisDeclarationADefinition());
+ }
+}
+
INSTANTIATE_TEST_CASE_P(
ParameterizedTests, ASTImporterTestBase,
::testing::Values(ArgVector(), ArgVector{"-fdelayed-template-parsing"}),);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46353: [ASTImporter] Extend lookup logic in class templates
This revision was automatically updated to reflect the committed changes.
Closed by commit rC332338: [ASTImporter] Extend lookup logic in class templates
(authored by a.sidorin, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D46353?vs=146765&id=146780#toc
Repository:
rC Clang
https://reviews.llvm.org/D46353
Files:
lib/AST/ASTImporter.cpp
unittests/AST/ASTImporterTest.cpp
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -4108,8 +4108,14 @@
if (auto *FoundTemplate = dyn_cast(Found)) {
if (IsStructuralMatch(D, FoundTemplate)) {
// The class templates structurally match; call it the same template.
- // FIXME: We may be filling in a forward declaration here. Handle
- // this case!
+
+ // We found a forward declaration but the class to be imported has a
+ // definition.
+ // FIXME Add this forward declaration to the redeclaration chain.
+ if (D->isThisDeclarationADefinition() &&
+ !FoundTemplate->isThisDeclarationADefinition())
+continue;
+
Importer.Imported(D->getTemplatedDecl(),
FoundTemplate->getTemplatedDecl());
return Importer.Imported(D, FoundTemplate);
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1431,6 +1431,39 @@
MatchVerifier{}.match(To->getTranslationUnitDecl(), Pattern));
}
+TEST_P(ASTImporterTestBase, ImportDefinitionOfClassTemplateAfterFwdDecl) {
+ {
+Decl *FromTU = getTuDecl(
+R"(
+template
+struct B;
+)",
+Lang_CXX, "input0.cc");
+auto *FromD = FirstDeclMatcher().match(
+FromTU, classTemplateDecl(hasName("B")));
+
+Import(FromD, Lang_CXX);
+ }
+
+ {
+Decl *FromTU = getTuDecl(
+R"(
+template
+struct B {
+ void f();
+};
+)",
+Lang_CXX, "input1.cc");
+FunctionDecl *FromD = FirstDeclMatcher().match(
+FromTU, functionDecl(hasName("f")));
+Import(FromD, Lang_CXX);
+auto *FromCTD = FirstDeclMatcher().match(
+FromTU, classTemplateDecl(hasName("B")));
+auto *ToCTD = cast(Import(FromCTD, Lang_CXX));
+EXPECT_TRUE(ToCTD->isThisDeclarationADefinition());
+ }
+}
+
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
@@ -4108,8 +4108,14 @@
if (auto *FoundTemplate = dyn_cast(Found)) {
if (IsStructuralMatch(D, FoundTemplate)) {
// The class templates structurally match; call it the same template.
- // FIXME: We may be filling in a forward declaration here. Handle
- // this case!
+
+ // We found a forward declaration but the class to be imported has a
+ // definition.
+ // FIXME Add this forward declaration to the redeclaration chain.
+ if (D->isThisDeclarationADefinition() &&
+ !FoundTemplate->isThisDeclarationADefinition())
+continue;
+
Importer.Imported(D->getTemplatedDecl(),
FoundTemplate->getTemplatedDecl());
return Importer.Imported(D, FoundTemplate);
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1431,6 +1431,39 @@
MatchVerifier{}.match(To->getTranslationUnitDecl(), Pattern));
}
+TEST_P(ASTImporterTestBase, ImportDefinitionOfClassTemplateAfterFwdDecl) {
+ {
+Decl *FromTU = getTuDecl(
+R"(
+template
+struct B;
+)",
+Lang_CXX, "input0.cc");
+auto *FromD = FirstDeclMatcher().match(
+FromTU, classTemplateDecl(hasName("B")));
+
+Import(FromD, Lang_CXX);
+ }
+
+ {
+Decl *FromTU = getTuDecl(
+R"(
+template
+struct B {
+ void f();
+};
+)",
+Lang_CXX, "input1.cc");
+FunctionDecl *FromD = FirstDeclMatcher().match(
+FromTU, functionDecl(hasName("f")));
+Import(FromD, Lang_CXX);
+auto *FromCTD = FirstDeclMatcher().match(
+FromTU, classTemplateDecl(hasName("B")));
+auto *ToCTD = cast(Import(FromCTD, Lang_CXX));
+EXPECT_TRUE(ToCTD->isThisDeclarationADefinition());
+ }
+}
+
INSTANTIATE_TEST_CASE_P(
ParameterizedTests, ASTImporterTestBase,
::testing::Values(ArgVector(), ArgVector{"-fdelayed-template-parsing"}),);
___
[PATCH] D46871: [AMDGPU] Add interpolation builtins
timcorringham created this revision.
Herald added subscribers: cfe-commits, t-tye, tpr, dstuttard, yaxunl, nhaehnle,
wdng, kzhuravl.
Added builtins for the interpolation intrinsics, and related LIT
test.
Repository:
rC Clang
https://reviews.llvm.org/D46871
Files:
include/clang/Basic/BuiltinsAMDGPU.def
test/CodeGenOpenCL/builtins-amdgcn-interp.cl
Index: test/CodeGenOpenCL/builtins-amdgcn-interp.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/builtins-amdgcn-interp.cl
@@ -0,0 +1,44 @@
+// REQUIRES: amdgpu-registered-target
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu gfx900 -S -o -
%s | FileCheck %s --check-prefixes=CHECK,GFX9,BANK32
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu fiji -S -o - %s
| FileCheck %s --check-prefixes=CHECK,GFX8,BANK32
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu gfx810 -S -o -
%s | FileCheck %s --check-prefixes=CHECK,GFX8,BANK16
+
+#pragma OPENCL EXTENSION cl_khr_fp16 : enable
+
+// CHECK-LABEL: test_interp_f16
+// CHECK: s_mov_b32 m0, s{{[0-9]}}
+// BANK32: v_interp_p1ll_f16 v{{[0-9]}}, v{{[0-9]}}, attr3.z{{$}}
+// BANK32: v_interp_p1ll_f16 v{{[0-9]}}, v{{[0-9]}}, attr3.z high
+// BANK16: v_interp_mov_f32_e32 v4, p0, attr3.z
+// BANK16: v_interp_p1lv_f16 v{{[0-9]}}, v{{[0-9]}}, attr3.z, v{{[0-9]}}{{$}}
+// BANK16: v_interp_p1lv_f16 v{{[0-9]}}, v{{[0-9]}}, attr3.z, v{{[0-9]}} high
+// GFX9: _interp_p2_legacy_f16 v{{[0-9]}}, v{{[0-9]}}, attr3.z, v{{[0-9]}}{{$}}
+// GFX9: v_interp_p2_legacy_f16 v{{[0-9]}}, v{{[0-9]}}, attr3.z, v{{[0-9]}}
high
+// GFX8: _interp_p2_f16 v{{[0-9]}}, v{{[0-9]}}, attr3.z, v{{[0-9]}}{{$}}
+// GFX8: v_interp_p2_f16 v{{[0-9]}}, v{{[0-9]}}, attr3.z, v{{[0-9]}} high
+// CHECK: v_add_f16_e32 v{{[0-9]}}, v{{[0-9]}}, v{{[0-9]}}
+void test_interp_f16(global half* out, float i, float j, int m0)
+{
+ float p1_0 = __builtin_amdgcn_interp_p1_f16(i, 2, 3, false, m0);
+ half p2_0 = __builtin_amdgcn_interp_p2_f16(p1_0, j, 2, 3, false, m0);
+ float p1_1 = __builtin_amdgcn_interp_p1_f16(i, 2, 3, true, m0);
+ half p2_1 = __builtin_amdgcn_interp_p2_f16(p1_1, j, 2, 3, true, m0);
+ *out = p2_0 + p2_1;
+}
+
+// CHECK-LABEL: test_interp_f32
+// CHECK: s_mov_b32 m0, s{{[0-9]}}
+// CHECK: v_interp_p1_f32_e32 v{{[0-9]}}, v{{[0-9]}}, attr4.y
+// CHECK: v_interp_p2_f32_e32 v{{[0-9]}}, v{{[0-9]}}, attr4.y
+void test_interp_f32(global float* out, float i, float j, int m0)
+{
+ float p1 = __builtin_amdgcn_interp_p1(i, 1, 4, m0);
+ *out = __builtin_amdgcn_interp_p2(p1, j, 1, 4, m0);
+}
+
+// CHECK-LABEL: test_interp_mov
+// CHECK: v_interp_mov_f32_e32 v{{[0-9]}}, p0, attr4.w
+void test_interp_mov(global float* out, float i, float j, int m0)
+{
+ *out = __builtin_amdgcn_interp_mov(2, 3, 4, m0);
+}
Index: include/clang/Basic/BuiltinsAMDGPU.def
===
--- include/clang/Basic/BuiltinsAMDGPU.def
+++ include/clang/Basic/BuiltinsAMDGPU.def
@@ -98,6 +98,15 @@
BUILTIN(__builtin_amdgcn_ds_fmax, "ff*3fiib", "n")
//===--===//
+// Interpolation builtins.
+//===--===//
+BUILTIN(__builtin_amdgcn_interp_p1_f16, "ffUiUibUi", "nc")
+BUILTIN(__builtin_amdgcn_interp_p2_f16, "hffUiUibUi", "nc")
+BUILTIN(__builtin_amdgcn_interp_p1, "ffUiUiUi", "nc")
+BUILTIN(__builtin_amdgcn_interp_p2, "fffUiUiUi", "nc")
+BUILTIN(__builtin_amdgcn_interp_mov, "fUiUiUiUi", "nc")
+
+//===--===//
// VI+ only builtins.
//===--===//
Index: test/CodeGenOpenCL/builtins-amdgcn-interp.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/builtins-amdgcn-interp.cl
@@ -0,0 +1,44 @@
+// REQUIRES: amdgpu-registered-target
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu gfx900 -S -o - %s | FileCheck %s --check-prefixes=CHECK,GFX9,BANK32
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu fiji -S -o - %s | FileCheck %s --check-prefixes=CHECK,GFX8,BANK32
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu gfx810 -S -o - %s | FileCheck %s --check-prefixes=CHECK,GFX8,BANK16
+
+#pragma OPENCL EXTENSION cl_khr_fp16 : enable
+
+// CHECK-LABEL: test_interp_f16
+// CHECK: s_mov_b32 m0, s{{[0-9]}}
+// BANK32: v_interp_p1ll_f16 v{{[0-9]}}, v{{[0-9]}}, attr3.z{{$}}
+// BANK32: v_interp_p1ll_f16 v{{[0-9]}}, v{{[0-9]}}, attr3.z high
+// BANK16: v_interp_mov_f32_e32 v4, p0, attr3.z
+// BANK16: v_interp_p1lv_f16 v{{[0-9]}}, v{{[0-9]}}, attr3.z, v{{[0-9]}}{{$}}
+// BANK16: v_interp_p1lv_f16 v{{[0-9]}}, v{{[0-9]}}, attr3.z, v{{[0-9]}} high
+// GFX9: _interp_p2_legacy_f16 v{{[0-9]}}, v{{[0-9]}}, attr3.z, v{{[0-9]}}{{$}}
+// GFX9: v_interp_p2_legacy_f16 v{{[0-9]}}, v{{[0-9]}}, attr3.z, v{{[0-9]}} h
[PATCH] D41241: [Solaris] Only define _REENTRANT if -pthread
This revision was automatically updated to reflect the committed changes.
Closed by commit rC332343: [Solaris] Only define _REENTRANT if -pthread
(authored by ro, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D41241
Files:
lib/Basic/Targets/OSTargets.h
Index: lib/Basic/Targets/OSTargets.h
===
--- lib/Basic/Targets/OSTargets.h
+++ lib/Basic/Targets/OSTargets.h
@@ -551,7 +551,8 @@
Builder.defineMacro("_LARGEFILE_SOURCE");
Builder.defineMacro("_LARGEFILE64_SOURCE");
Builder.defineMacro("__EXTENSIONS__");
-Builder.defineMacro("_REENTRANT");
+if (Opts.POSIXThreads)
+ Builder.defineMacro("_REENTRANT");
if (this->HasFloat128)
Builder.defineMacro("__FLOAT128__");
}
Index: lib/Basic/Targets/OSTargets.h
===
--- lib/Basic/Targets/OSTargets.h
+++ lib/Basic/Targets/OSTargets.h
@@ -551,7 +551,8 @@
Builder.defineMacro("_LARGEFILE_SOURCE");
Builder.defineMacro("_LARGEFILE64_SOURCE");
Builder.defineMacro("__EXTENSIONS__");
-Builder.defineMacro("_REENTRANT");
+if (Opts.POSIXThreads)
+ Builder.defineMacro("_REENTRANT");
if (this->HasFloat128)
Builder.defineMacro("__FLOAT128__");
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D10833: Retrieve BinaryOperator::getOpcode and BinaryOperator::getOpcodeStr via libclang and its python interface
kent08ian added a comment. In https://reviews.llvm.org/D10833#970906, @milianw wrote: > still looks good to me. can someone else please review and commit this? Ping Repository: rC Clang https://reviews.llvm.org/D10833 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46497: [clangd] Populate #include insertions as additional edits in completion items.
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
Nice, ship it!
Comment at: clangd/CodeComplete.cpp:317
+if (Includes && !D->IncludeHeader.empty()) {
+ // Fallback to canonical header if declaration location is invalid.
+ auto Edit = [&]() -> Expected> {
is this comment still relevant here?
Comment at: clangd/CodeComplete.cpp:792
+if (!Style) {
+ log("Failed to get FormaStyle for file" + Input.FileName +
+ ". Fall back to use LLVM style. Error: " +
nit: FormatStyle
Comment at: clangd/CodeComplete.cpp:803
+
Clang->getPreprocessor().addPPCallbacks(collectInclusionsInMainFileCallback(
+Clang->getSourceManager(), [&Includes](Inclusion Inc) {
+ Includes->get()->addExisting(std::move(Inc));
nit: includes is a pointer, capture by value?
Comment at: clangd/CodeComplete.cpp:962
Output = runWithSema();
+ Includes.release(); // Make sure this doesn't out-live Clang.
SPAN_ATTACH(Tracer, "sema_completion_kind",
This leaks. reset (or = nullptr)
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D46497
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D42966: Fix USR generation in the presence of #line directives or linemarkes
> > > Could you provide a minimal example where USRs are not generated? It might > be the case that there are other ways to fix it. > > Sure, I'll try to reduce our testcase, but basically we have an ASTFrontendAction [0] that adds a set of intrinsics [1] to the preprocessor [2]. [0] https://github.com/esbmc/esbmc/blob/master/src/clang-c-frontend/AST/esbmc_action.h [1] https://github.com/esbmc/esbmc/blob/master/src/clang-c-frontend/clang_c_language.cpp#L206 [2] https://github.com/esbmc/esbmc/blob/master/src/clang-c-frontend/AST/esbmc_action.h#L31 -- Mikhail Ramalho. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r332296 - [CodeView] Improve debugging of virtual base class member variables
Author: bwyma
Date: Mon May 14 14:21:22 2018
New Revision: 332296
URL: http://llvm.org/viewvc/llvm-project?rev=332296&view=rev
Log:
[CodeView] Improve debugging of virtual base class member variables
Initial support for passing the virtual base pointer offset to CodeViewDebug.
https://reviews.llvm.org/D46271
Modified:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/test/CodeGenCXX/debug-info-access.cpp
cfe/trunk/test/CodeGenCXX/debug-info-ms-vbase.cpp
Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=332296&r1=332295&r2=332296&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Mon May 14 14:21:22 2018
@@ -1536,6 +1536,7 @@ void CGDebugInfo::CollectCXXBasesAux(
auto *BaseTy = getOrCreateType(BI.getType(), Unit);
llvm::DINode::DIFlags BFlags = StartingFlags;
uint64_t BaseOffset;
+uint32_t VBPtrOffset = 0;
if (BI.isVirtual()) {
if (CGM.getTarget().getCXXABI().isItaniumFamily()) {
@@ -1549,6 +1550,8 @@ void CGDebugInfo::CollectCXXBasesAux(
// vbase offset offset in Itanium.
BaseOffset =
4 * CGM.getMicrosoftVTableContext().getVBTableIndex(RD, Base);
+VBPtrOffset = CGM.getContext().getASTRecordLayout(RD).getVBPtrOffset()
+ .getQuantity();
}
BFlags |= llvm::DINode::FlagVirtual;
} else
@@ -1558,7 +1561,8 @@ void CGDebugInfo::CollectCXXBasesAux(
BFlags |= getAccessFlag(BI.getAccessSpecifier(), RD);
llvm::DIType *DTy =
-DBuilder.createInheritance(RecordTy, BaseTy, BaseOffset, BFlags);
+DBuilder.createInheritance(RecordTy, BaseTy, BaseOffset, VBPtrOffset,
+ BFlags);
EltTys.push_back(DTy);
}
}
@@ -2192,7 +2196,7 @@ llvm::DIType *CGDebugInfo::CreateTypeDef
if (!SClassTy)
return nullptr;
-llvm::DIType *InhTag = DBuilder.createInheritance(RealDecl, SClassTy, 0,
+llvm::DIType *InhTag = DBuilder.createInheritance(RealDecl, SClassTy, 0, 0,
llvm::DINode::FlagZero);
EltTys.push_back(InhTag);
}
Modified: cfe/trunk/test/CodeGenCXX/debug-info-access.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-access.cpp?rev=332296&r1=332295&r2=332296&view=diff
==
--- cfe/trunk/test/CodeGenCXX/debug-info-access.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/debug-info-access.cpp Mon May 14 14:21:22 2018
@@ -10,7 +10,7 @@ struct A {
};
-// CHECK: !DIDerivedType(tag: DW_TAG_inheritance,{{.*}} baseType:
![[A]],{{.*}} flags: DIFlagPublic)
+// CHECK: !DIDerivedType(tag: DW_TAG_inheritance,{{.*}} baseType:
![[A]],{{.*}} flags: DIFlagPublic, extraData: i32 0)
class B : public A {
public:
// CHECK-DAG: !DISubprogram(name: "pub",{{.*}} line: [[@LINE+1]],{{.*}}
flags: DIFlagPublic | DIFlagPrototyped,
Modified: cfe/trunk/test/CodeGenCXX/debug-info-ms-vbase.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-ms-vbase.cpp?rev=332296&r1=332295&r2=332296&view=diff
==
--- cfe/trunk/test/CodeGenCXX/debug-info-ms-vbase.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/debug-info-ms-vbase.cpp Mon May 14 14:21:22 2018
@@ -8,7 +8,7 @@
// CHECK: ![[elements]] = !{![[NoPrimaryBase_base:[0-9]+]]}
// CHECK: ![[NoPrimaryBase_base]] = !DIDerivedType(tag: DW_TAG_inheritance,
scope: ![[NoPrimaryBase]],
-// CHECK-SAME: baseType: ![[HasVirtualMethod:[0-9]+]], offset: 4, flags:
DIFlagVirtual)
+// CHECK-SAME: baseType: ![[HasVirtualMethod:[0-9]+]], offset: 4, flags:
DIFlagVirtual, extraData: i32 0)
// CHECK: ![[HasVirtualMethod]] = distinct !DICompositeType(tag:
DW_TAG_structure_type, name: "HasVirtualMethod"
@@ -18,11 +18,11 @@
// CHECK: ![[elements]] = !{![[SecondaryVTable_base:[0-9]+]],
![[HasVirtualMethod_base:[0-9]+]], ![[vshape:[0-9]+]]}
// CHECK: ![[SecondaryVTable_base]] = !DIDerivedType(tag: DW_TAG_inheritance,
scope: ![[HasPrimaryBase]],
-// CHECK-SAME: baseType: ![[SecondaryVTable:[0-9]+]], offset: 4, flags:
DIFlagVirtual)
+// CHECK-SAME: baseType: ![[SecondaryVTable:[0-9]+]], offset: 4, flags:
DIFlagVirtual, extraData: i32 4)
// CHECK: ![[SecondaryVTable]] = distinct !DICompositeType(tag:
DW_TAG_structure_type, name: "SecondaryVTable"
-// CHECK: ![[HasVirtualMethod_base]] = !DIDerivedType(tag: DW_TAG_inheritance,
scope: ![[HasPrimaryBase]], baseType: ![[HasVirtualMethod]])
+// CHECK: ![[HasVirtualMethod_base]] = !DIDerivedType(tag: DW_TAG_inheritance,
scope: ![[HasPrimaryBase]], baseType: ![[HasVirtualMethod]], extraData: i32 0)
// CHECK: ![[HasIndirectVirtualBase:[0-9]+]] = distinct !DICompositeType(tag:
DW_TAG_st
r332343 - [Solaris] Only define _REENTRANT if -pthread
Author: ro
Date: Tue May 15 04:36:00 2018
New Revision: 332343
URL: http://llvm.org/viewvc/llvm-project?rev=332343&view=rev
Log:
[Solaris] Only define _REENTRANT if -pthread
When looking at lib/Basic/Targets/OSTargets.h, I noticed that _REENTRANT is
defined
unconditionally on Solaris, unlike all other targets and what either Studio cc
(only define
it with -mt) or gcc (only define it with -pthread) do.
This patch follows that lead.
Differential Revision: https://reviews.llvm.org/D41241
Modified:
cfe/trunk/lib/Basic/Targets/OSTargets.h
Modified: cfe/trunk/lib/Basic/Targets/OSTargets.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/OSTargets.h?rev=332343&r1=332342&r2=332343&view=diff
==
--- cfe/trunk/lib/Basic/Targets/OSTargets.h (original)
+++ cfe/trunk/lib/Basic/Targets/OSTargets.h Tue May 15 04:36:00 2018
@@ -551,7 +551,8 @@ protected:
Builder.defineMacro("_LARGEFILE_SOURCE");
Builder.defineMacro("_LARGEFILE64_SOURCE");
Builder.defineMacro("__EXTENSIONS__");
-Builder.defineMacro("_REENTRANT");
+if (Opts.POSIXThreads)
+ Builder.defineMacro("_REENTRANT");
if (this->HasFloat128)
Builder.defineMacro("__FLOAT128__");
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r332286 - PR37450: Fix bug that disabled some type checks for variables with deduced types.
Hi Richard,
The for-range-examples.cpp test fails on 32-bit arm buildbots, e.g.:
http://lab.llvm.org:8011/builders/clang-cmake-armv7-full/builds/840 . Would
you please investigate?
You didn't get a notification because your commit was around the same time as a
fix for an unrelated testcase issue that caused same bots to be red.
--
Maxim Kuvyrkov
www.linaro.org
> On May 14, 2018, at 11:15 PM, Richard Smith via cfe-commits
> wrote:
>
> Author: rsmith
> Date: Mon May 14 13:15:04 2018
> New Revision: 332286
>
> URL: http://llvm.org/viewvc/llvm-project?rev=332286&view=rev
> Log:
> PR37450: Fix bug that disabled some type checks for variables with deduced
> types.
>
> Also improve diagnostic for the case where a type is non-literal because it's
> a lambda.
>
> Modified:
>cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>cfe/trunk/lib/Sema/SemaDecl.cpp
>cfe/trunk/lib/Sema/SemaType.cpp
>cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/p3.cpp
>cfe/trunk/test/SemaCXX/cxx1z-constexpr-lambdas.cpp
>cfe/trunk/test/SemaCXX/for-range-examples.cpp
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=332286&r1=332285&r2=332286&view=diff
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon May 14 13:15:04
> 2018
> @@ -2381,6 +2381,8 @@ def note_non_literal_user_provided_dtor
> "%0 is not literal because it has a user-provided destructor">;
> def note_non_literal_nontrivial_dtor : Note<
> "%0 is not literal because it has a non-trivial destructor">;
> +def note_non_literal_lambda : Note<
> + "lambda closure types are non-literal types before C++17">;
> def warn_private_extern : Warning<
> "use of __private_extern__ on a declaration may not produce external symbol
> "
> "private to the linkage unit and is deprecated">, InGroup;
>
> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=332286&r1=332285&r2=332286&view=diff
> ==
> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Mon May 14 13:15:04 2018
> @@ -7293,8 +7293,7 @@ void Sema::CheckVariableDeclarationType(
> if (NewVD->isInvalidDecl())
> return;
>
> - TypeSourceInfo *TInfo = NewVD->getTypeSourceInfo();
> - QualType T = TInfo->getType();
> + QualType T = NewVD->getType();
>
> // Defer checking an 'auto' type until its initializer is attached.
> if (T->isUndeducedType())
> @@ -7438,10 +7437,18 @@ void Sema::CheckVariableDeclarationType(
> (T->isVariableArrayType() && NewVD->hasGlobalStorage())) {
> bool SizeIsNegative;
> llvm::APSInt Oversized;
> -TypeSourceInfo *FixedTInfo =
> - TryToFixInvalidVariablyModifiedTypeSourceInfo(TInfo, Context,
> -SizeIsNegative,
> Oversized);
> -if (!FixedTInfo && T->isVariableArrayType()) {
> +TypeSourceInfo *FixedTInfo =
> TryToFixInvalidVariablyModifiedTypeSourceInfo(
> +NewVD->getTypeSourceInfo(), Context, SizeIsNegative, Oversized);
> +QualType FixedT;
> +if (FixedTInfo && T == NewVD->getTypeSourceInfo()->getType())
> + FixedT = FixedTInfo->getType();
> +else if (FixedTInfo) {
> + // Type and type-as-written are canonically different. We need to fix
> up
> + // both types separately.
> + FixedT = TryToFixInvalidVariablyModifiedType(T, Context,
> SizeIsNegative,
> + Oversized);
> +}
> +if ((!FixedTInfo || FixedT.isNull()) && T->isVariableArrayType()) {
> const VariableArrayType *VAT = Context.getAsVariableArrayType(T);
> // FIXME: This won't give the correct result for
> // int a[10][n];
> @@ -7470,7 +7477,7 @@ void Sema::CheckVariableDeclarationType(
> }
>
> Diag(NewVD->getLocation(), diag::warn_illegal_constant_array_size);
> -NewVD->setType(FixedTInfo->getType());
> +NewVD->setType(FixedT);
> NewVD->setTypeSourceInfo(FixedTInfo);
> }
>
>
> Modified: cfe/trunk/lib/Sema/SemaType.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=332286&r1=332285&r2=332286&view=diff
> ==
> --- cfe/trunk/lib/Sema/SemaType.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaType.cpp Mon May 14 13:15:04 2018
> @@ -7809,6 +7809,13 @@ bool Sema::RequireLiteralType(SourceLoca
> if (RequireCompleteType(Loc, ElemType, diag::note_non_literal_incomplete,
> T))
> return true;
>
> + // [expr.prim.lambda]p3:
> + // This class type is [not] a literal type.
> + if (RD->isLambda() && !getLangOpts().CPlusP
[PATCH] D46386: Adding __atomic_fetch_min/max intrinsics to clang
algrant added a comment. There is a (stalled) proposal to add atomic integer max/min to C++: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0493r0.pdf . The proposal has memory semantics similar to these builtins, i.e. unconditional RMW. There is no limitation to 32-bit types though, what's the motivation for that? Is it a limitation of a particular target? Repository: rL LLVM https://reviews.llvm.org/D46386 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46751: [clangd] Filter out private proto symbols in SymbolCollector.
sammccall added a comment. @malaperle to expand on what Eric said, adding proto hacks with false positives and no way to turn them off is indeed not the way to go here! There's probably going to be other places we want to filter symbols too, and it should probably be extensible/customizable in some way. We don't yet have enough examples to know what the structure should be (something regex based, a code-plugin system based on `Registry`, or something in between), thus the simplest/least invasive option for now (it's important for our internal rollout to have *some* mitigation in place). @ioeric can you add a comment near the proto-filtering stuff indicating we should work out how to make this extensible? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46751 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer
r.stahl updated this revision to Diff 146789.
r.stahl marked 3 inline comments as done.
r.stahl edited the summary of this revision.
r.stahl added a comment.
Herald added subscribers: whisperity, mgorny.
I looked through the original patches and found quite a few more occurrences of
"function map" and renamed them - including the tool "clang-func-mapping".
There is one comment "by using the 'clang-extdef-mapping' packaged with Xcode
(on OS X)". Is this implicit from the install targets that the tool name
changed or do I have to inform someone?
https://reviews.llvm.org/D46421
Files:
include/clang/Basic/DiagnosticCrossTUKinds.td
include/clang/CrossTU/CrossTranslationUnit.h
include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
lib/CrossTU/CrossTranslationUnit.cpp
lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
test/Analysis/Inputs/ctu-other.cpp
test/Analysis/Inputs/externalDefMap.txt
test/Analysis/Inputs/externalFnMap.txt
test/Analysis/ctu-main.cpp
test/Analysis/func-mapping-test.cpp
test/CMakeLists.txt
test/lit.cfg.py
tools/CMakeLists.txt
tools/clang-extdef-mapping/CMakeLists.txt
tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
tools/clang-func-mapping/CMakeLists.txt
tools/clang-func-mapping/ClangFnMapGen.cpp
tools/scan-build-py/README.md
tools/scan-build-py/libscanbuild/__init__.py
tools/scan-build-py/libscanbuild/analyze.py
tools/scan-build-py/libscanbuild/arguments.py
tools/scan-build-py/libscanbuild/clang.py
tools/scan-build-py/tests/unit/test_analyze.py
tools/scan-build-py/tests/unit/test_clang.py
Index: tools/scan-build-py/tests/unit/test_clang.py
===
--- tools/scan-build-py/tests/unit/test_clang.py
+++ tools/scan-build-py/tests/unit/test_clang.py
@@ -96,7 +96,7 @@
class ClangIsCtuCapableTest(unittest.TestCase):
def test_ctu_not_found(self):
-is_ctu = sut.is_ctu_capable('not-found-clang-func-mapping')
+is_ctu = sut.is_ctu_capable('not-found-clang-extdef-mapping')
self.assertFalse(is_ctu)
Index: tools/scan-build-py/tests/unit/test_analyze.py
===
--- tools/scan-build-py/tests/unit/test_analyze.py
+++ tools/scan-build-py/tests/unit/test_analyze.py
@@ -349,14 +349,14 @@
class MergeCtuMapTest(unittest.TestCase):
def test_no_map_gives_empty(self):
-pairs = sut.create_global_ctu_function_map([])
+pairs = sut.create_global_ctu_extdef_map([])
self.assertFalse(pairs)
def test_multiple_maps_merged(self):
concat_map = ['c:@F@fun1#I# ast/fun1.c.ast',
'c:@F@fun2#I# ast/fun2.c.ast',
'c:@F@fun3#I# ast/fun3.c.ast']
-pairs = sut.create_global_ctu_function_map(concat_map)
+pairs = sut.create_global_ctu_extdef_map(concat_map)
self.assertTrue(('c:@F@fun1#I#', 'ast/fun1.c.ast') in pairs)
self.assertTrue(('c:@F@fun2#I#', 'ast/fun2.c.ast') in pairs)
self.assertTrue(('c:@F@fun3#I#', 'ast/fun3.c.ast') in pairs)
@@ -366,7 +366,7 @@
concat_map = ['c:@F@fun1#I# ast/fun1.c.ast',
'c:@F@fun2#I# ast/fun2.c.ast',
'c:@F@fun1#I# ast/fun7.c.ast']
-pairs = sut.create_global_ctu_function_map(concat_map)
+pairs = sut.create_global_ctu_extdef_map(concat_map)
self.assertFalse(('c:@F@fun1#I#', 'ast/fun1.c.ast') in pairs)
self.assertFalse(('c:@F@fun1#I#', 'ast/fun7.c.ast') in pairs)
self.assertTrue(('c:@F@fun2#I#', 'ast/fun2.c.ast') in pairs)
@@ -376,28 +376,28 @@
concat_map = ['c:@F@fun1#I# ast/fun1.c.ast',
'c:@F@fun2#I# ast/fun2.c.ast',
'c:@F@fun1#I# ast/fun1.c.ast']
-pairs = sut.create_global_ctu_function_map(concat_map)
+pairs = sut.create_global_ctu_extdef_map(concat_map)
self.assertTrue(('c:@F@fun1#I#', 'ast/fun1.c.ast') in pairs)
self.assertTrue(('c:@F@fun2#I#', 'ast/fun2.c.ast') in pairs)
self.assertEqual(2, len(pairs))
def test_space_handled_in_source(self):
concat_map = ['c:@F@fun1#I# ast/f un.c.ast']
-pairs = sut.create_global_ctu_function_map(concat_map)
+pairs = sut.create_global_ctu_extdef_map(concat_map)
self.assertTrue(('c:@F@fun1#I#', 'ast/f un.c.ast') in pairs)
self.assertEqual(1, len(pairs))
-class FuncMapSrcToAstTest(unittest.TestCase):
+class ExtdefMapSrcToAstTest(unittest.TestCase):
def test_empty_gives_empty(self):
-fun_ast_lst = sut.func_map_list_src_to_ast([])
+fun_ast_lst = sut.extdef_map_list_src_to_ast([])
self.assertFalse(fun_ast_lst)
def test_sources_to_asts(self):
fun_src_lst = ['c:@F@f1#I# ' + os.path.join(os.sep + 'path', 'f1.c'),
'c:@F@f2#I# ' + os.path.join(os.sep + 'path', 'f2.c')]
-fun_ast_lst = sut.func_map_list_src_to_ast(fun_src_lst)
+
[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer
r.stahl added inline comments. Comment at: include/clang/CrossTU/CrossTranslationUnit.h:114 + llvm::Expected + getCrossTUDefinition(const VarDecl *VD, StringRef CrossTUDir, + StringRef IndexName); xazax.hun wrote: > I wonder if we need these overloads. Maybe having only the templated version > and having a static assert for the supported types is better? Asking the > other reviewers as well. They are not required, but I thought they make the interface more clear and help prevent implementation in headers. Comment at: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:354 +if (!VD->hasExternalStorage() || +VD->getAnyInitializer() != nullptr) + return true; xazax.hun wrote: > Will this work for zero initialized globals (i.e. not having an initializer > expression) that are defined in the current translation unit? It would be > great to have a test case for that. If a variable has a definition in the current TU, it may not have another one else where, but you're correct that this case will continue here. It will however only result in a failed lookup. Similar but backwards: If an extern var is defined in another TU with global zero init. Overall I'd say that default initializing a constant to zero is pretty uncommon (and invalid in C++), so that in my opinion it's fine to not support it for now. It seems like there is no transparent way to get the initializer including that case or am I missing something? https://reviews.llvm.org/D46421 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r332350 - [clang] Update uses of DEBUG macro to LLVM_DEBUG.
Author: nzaghen
Date: Tue May 15 06:30:56 2018
New Revision: 332350
URL: http://llvm.org/viewvc/llvm-project?rev=332350&view=rev
Log:
[clang] Update uses of DEBUG macro to LLVM_DEBUG.
The DEBUG() macro is very generic so it might clash with other projects.
The renaming was done as follows:
- git grep -l 'DEBUG' | xargs sed -i 's/\bDEBUG\s\?(/LLVM_DEBUG(/g'
- git diff -U0 master | ../clang/tools/clang-format/clang-format-diff.py -i -p1
-style LLVM
Explicitly avoided changing the strings in the clang-format tests.
Differential Revision: https://reviews.llvm.org/D44975
Modified:
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/Analysis/BodyFarm.cpp
cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
cfe/trunk/lib/Format/BreakableToken.cpp
cfe/trunk/lib/Format/ContinuationIndenter.cpp
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/lib/Format/SortJavaScriptImports.cpp
cfe/trunk/lib/Format/TokenAnalyzer.cpp
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
cfe/trunk/lib/Format/UnwrappedLineParser.cpp
cfe/trunk/lib/Format/UsingDeclarationsSorter.cpp
cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
cfe/trunk/lib/Tooling/Tooling.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
cfe/trunk/unittests/Format/FormatTestComments.cpp
cfe/trunk/unittests/Format/FormatTestJS.cpp
cfe/trunk/unittests/Format/FormatTestJava.cpp
cfe/trunk/unittests/Format/FormatTestObjC.cpp
cfe/trunk/unittests/Format/FormatTestProto.cpp
cfe/trunk/unittests/Format/FormatTestRawStrings.cpp
cfe/trunk/unittests/Format/FormatTestSelective.cpp
cfe/trunk/unittests/Format/FormatTestTextProto.cpp
cfe/trunk/unittests/Format/NamespaceEndCommentsFixerTest.cpp
cfe/trunk/unittests/Format/UsingDeclarationsSorterTest.cpp
cfe/trunk/unittests/libclang/LibclangTest.cpp
Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=332350&r1=332349&r2=332350&view=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Tue May 15 06:30:56 2018
@@ -6961,8 +6961,8 @@ bool ArrayExprEvaluator::VisitInitListEx
if (NumEltsToInit != NumElts && MaybeElementDependentArrayFiller(FillerExpr))
NumEltsToInit = NumElts;
- DEBUG(llvm::dbgs() << "The number of elements to initialize: " <<
-NumEltsToInit << ".\n");
+ LLVM_DEBUG(llvm::dbgs() << "The number of elements to initialize: "
+ << NumEltsToInit << ".\n");
Result = APValue(APValue::UninitArray(), NumEltsToInit, NumElts);
Modified: cfe/trunk/lib/Analysis/BodyFarm.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/BodyFarm.cpp?rev=332350&r1=332349&r2=332350&view=diff
==
--- cfe/trunk/lib/Analysis/BodyFarm.cpp (original)
+++ cfe/trunk/lib/Analysis/BodyFarm.cpp Tue May 15 06:30:56 2018
@@ -314,7 +314,7 @@ static CallExpr *create_call_once_lambda
/// }
/// \endcode
static Stmt *create_call_once(ASTContext &C, const FunctionDecl *D) {
- DEBUG(llvm::dbgs() << "Generating body for call_once\n");
+ LLVM_DEBUG(llvm::dbgs() << "Generating body for call_once\n");
// We need at least two parameters.
if (D->param_size() < 2)
@@ -342,9 +342,9 @@ static Stmt *create_call_once(ASTContext
auto *FlagRecordDecl =
dyn_cast_or_null(FlagType->getAsTagDecl());
if (!FlagRecordDecl) {
-DEBUG(llvm::dbgs() << "Flag field is not a record: "
- << "unknown std::call_once implementation, "
- << "ignoring the call.\n");
+LLVM_DEBUG(llvm::dbgs() << "Flag field is not a record: "
+<< "unknown std::call_once implementation, "
+<< "ignoring the call.\n");
return nullptr;
}
@@ -359,16 +359,17 @@ static Stmt *create_call_once(ASTContext
}
if (!FlagFieldDecl) {
-DEBUG(llvm::dbgs() << "No field _M_once or __state_ found on "
- << "std::once_flag struct: unknown std::call_once "
- << "implementation, ignoring the call.");
+LLVM_DEBUG(llvm::dbgs() << "No field _M_once or __state_ found on "
+<< "std::once_flag struct: unknown std::call_once "
+<< "implementation, ignoring the call.");
return nullptr;
}
bool isLambdaCall = CallbackRecordDecl && CallbackRecordDecl->isLambda();
if (CallbackRecordDecl && !isLambdaCall) {
-DEBUG(llvm::dbgs() << "Not supported: synthesizing body for functors when "
- << "body farming std::call_once, ignoring the call.");
+LLVM_DEBUG(llvm::dbgs()
+ << "Not supported: synthesizing body for functors when "
+
[PATCH] D44975: Change DEBUG() macro to LLVM_DEBUG()
This revision was automatically updated to reflect the committed changes.
Closed by commit rL332350: [clang] Update uses of DEBUG macro to LLVM_DEBUG.
(authored by nzaghen, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D44975?vs=140055&id=146806#toc
Repository:
rL LLVM
https://reviews.llvm.org/D44975
Files:
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/Analysis/BodyFarm.cpp
cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
cfe/trunk/lib/Format/BreakableToken.cpp
cfe/trunk/lib/Format/ContinuationIndenter.cpp
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/lib/Format/SortJavaScriptImports.cpp
cfe/trunk/lib/Format/TokenAnalyzer.cpp
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
cfe/trunk/lib/Format/UnwrappedLineParser.cpp
cfe/trunk/lib/Format/UsingDeclarationsSorter.cpp
cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
cfe/trunk/lib/Tooling/Tooling.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
cfe/trunk/unittests/Format/FormatTestComments.cpp
cfe/trunk/unittests/Format/FormatTestJS.cpp
cfe/trunk/unittests/Format/FormatTestJava.cpp
cfe/trunk/unittests/Format/FormatTestObjC.cpp
cfe/trunk/unittests/Format/FormatTestProto.cpp
cfe/trunk/unittests/Format/FormatTestRawStrings.cpp
cfe/trunk/unittests/Format/FormatTestSelective.cpp
cfe/trunk/unittests/Format/FormatTestTextProto.cpp
cfe/trunk/unittests/Format/NamespaceEndCommentsFixerTest.cpp
cfe/trunk/unittests/Format/UsingDeclarationsSorterTest.cpp
cfe/trunk/unittests/libclang/LibclangTest.cpp
Index: cfe/trunk/lib/Tooling/Tooling.cpp
===
--- cfe/trunk/lib/Tooling/Tooling.cpp
+++ cfe/trunk/lib/Tooling/Tooling.cpp
@@ -473,7 +473,7 @@
// FIXME: We need a callback mechanism for the tool writer to output a
// customized message for each file.
- DEBUG({ llvm::dbgs() << "Processing: " << File << ".\n"; });
+ LLVM_DEBUG({ llvm::dbgs() << "Processing: " << File << ".\n"; });
ToolInvocation Invocation(std::move(CommandLine), Action, Files.get(),
PCHContainerOps);
Invocation.setDiagnosticConsumer(DiagConsumer);
Index: cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
===
--- cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
+++ cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
@@ -290,7 +290,7 @@
else
ASTSym->setSection("__clangast");
-DEBUG({
+LLVM_DEBUG({
// Print the IR for the PCH container to the debug output.
llvm::SmallString<0> Buffer;
clang::EmitBackendOutput(
Index: cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -1244,8 +1244,8 @@
if (!Overflow)
Overflow = checkedMul(Mult, offset.getQuantity());
if (Overflow) {
- DEBUG(llvm::dbgs() << "MemRegion::getAsArrayOffset: "
- << "offset overflowing, returning unknown\n");
+ LLVM_DEBUG(llvm::dbgs() << "MemRegion::getAsArrayOffset: "
+ << "offset overflowing, returning unknown\n");
return nullptr;
}
Index: cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -399,10 +399,10 @@
getManager()->getContext(FD);
bool IsAutosynthesized;
Stmt* Body = AD->getBody(IsAutosynthesized);
- DEBUG({
- if (IsAutosynthesized)
-llvm::dbgs() << "Using autosynthesized body for " << FD->getName()
- << "\n";
+ LLVM_DEBUG({
+if (IsAutosynthesized)
+ llvm::dbgs() << "Using autosynthesized body for " << FD->getName()
+ << "\n";
});
if (Body) {
const Decl* Decl = AD->getDecl();
Index: cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
@@ -905,7 +905,8 @@
Penalty = Queue.top().first.first;
StateNode *Node = Queue.top().second;
if (!Node->State.NextToken) {
-DEBUG(llvm::dbgs() << "\n---\nPenalty for line: " << Penalty << "\n");
+LLVM_DEBUG(llvm::dbgs()
+ << "\n---\nPenalty for line: " << Penalty << "\n");
break;
}
Queue.pop();
@@ -929,16 +930,17 @@
if (Queue.empty()) {
// We were unable to find a solution, do nothing.
// FIXME: Add diagnostic?
- DEBUG(llvm::dbgs
[PATCH] D46823: [analyzer] const init: handle non-explicit cases more accurately
r.stahl updated this revision to Diff 146809.
r.stahl marked 2 inline comments as done.
r.stahl added a comment.
updated test
https://reviews.llvm.org/D46823
Files:
lib/StaticAnalyzer/Core/RegionStore.cpp
test/Analysis/initialization.c
Index: test/Analysis/initialization.c
===
--- test/Analysis/initialization.c
+++ test/Analysis/initialization.c
@@ -1,7 +1,28 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
-// expected-no-diagnostics
+// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze
-analyzer-checker=core.builtin,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(int);
void initbug() {
const union { float a; } u = {};
(void)u.a; // no-crash
}
+
+int const parr[2] = {1};
+void constarr() {
+ int i = 2;
+ clang_analyzer_eval(parr[i]); // expected-warning{{UNDEFINED}}
+ i = 1;
+ clang_analyzer_eval(parr[i] == 0); // expected-warning{{TRUE}}
+ i = -1;
+ clang_analyzer_eval(parr[i]); // expected-warning{{UNDEFINED}}
+}
+
+struct SM {
+ int a;
+ int b;
+};
+const struct SM sm = {.a = 1};
+void multinit() {
+ clang_analyzer_eval(sm.a == 1); // expected-warning{{TRUE}}
+ clang_analyzer_eval(sm.b == 0); // expected-warning{{TRUE}}
+}
Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1638,9 +1638,18 @@
// The array index has to be known.
if (auto CI = R->getIndex().getAs()) {
int64_t i = CI->getValue().getSExtValue();
-// Return unknown value if index is out of bounds.
-if (i < 0 || i >= InitList->getNumInits())
- return UnknownVal();
+// If it is known that the index is out of bounds, we can return
+// an undefined value.
+if (i < 0)
+ return UndefinedVal();
+
+if (auto CAT = Ctx.getAsConstantArrayType(VD->getType()))
+ if (CAT->getSize().sle(i))
+return UndefinedVal();
+
+// If there is a list, but no init, it must be zero.
+if (i >= InitList->getNumInits())
+ return svalBuilder.makeZeroVal(R->getElementType());
if (const Expr *ElemInit = InitList->getInit(i))
if (Optional V = svalBuilder.getConstantVal(ElemInit))
@@ -1715,11 +1724,15 @@
// Either the record variable or the field has to be const qualified.
if (RecordVarTy.isConstQualified() || Ty.isConstQualified())
if (const Expr *Init = VD->getInit())
-if (const auto *InitList = dyn_cast(Init))
- if (Index < InitList->getNumInits())
+if (const auto *InitList = dyn_cast(Init)) {
+ if (Index < InitList->getNumInits()) {
if (const Expr *FieldInit = InitList->getInit(Index))
if (Optional V = svalBuilder.getConstantVal(FieldInit))
return *V;
+ } else {
+return svalBuilder.makeZeroVal(Ty);
+ }
+}
}
return getBindingForFieldOrElementCommon(B, R, Ty);
Index: test/Analysis/initialization.c
===
--- test/Analysis/initialization.c
+++ test/Analysis/initialization.c
@@ -1,7 +1,28 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
-// expected-no-diagnostics
+// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze -analyzer-checker=core.builtin,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(int);
void initbug() {
const union { float a; } u = {};
(void)u.a; // no-crash
}
+
+int const parr[2] = {1};
+void constarr() {
+ int i = 2;
+ clang_analyzer_eval(parr[i]); // expected-warning{{UNDEFINED}}
+ i = 1;
+ clang_analyzer_eval(parr[i] == 0); // expected-warning{{TRUE}}
+ i = -1;
+ clang_analyzer_eval(parr[i]); // expected-warning{{UNDEFINED}}
+}
+
+struct SM {
+ int a;
+ int b;
+};
+const struct SM sm = {.a = 1};
+void multinit() {
+ clang_analyzer_eval(sm.a == 1); // expected-warning{{TRUE}}
+ clang_analyzer_eval(sm.b == 0); // expected-warning{{TRUE}}
+}
Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1638,9 +1638,18 @@
// The array index has to be known.
if (auto CI = R->getIndex().getAs()) {
int64_t i = CI->getValue().getSExtValue();
-// Return unknown value if index is out of bounds.
-if (i < 0 || i >= InitList->getNumInits())
- return UnknownVal();
+// If it is known that the index is out of bounds, we can return
+// an undefined value.
+if (i < 0)
+ return UndefinedVal();
+
+if (auto CAT = Ctx.getAsConstantArrayType(VD->getType()))
[PATCH] D46823: [analyzer] const init: handle non-explicit cases more accurately
r.stahl added inline comments.
Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1650
+
+// If there is a list, but no init, it must be zero.
+if (i >= InitList->getNumInits())
NoQ wrote:
> NoQ wrote:
> > Would this work correctly if the element is not of an integral or
> > enumeration type? I think this needs an explicit check.
> What if we have an out-of-bounds access to a variable-length array? I don't
> think it'd yield zero.
I'm getting "variable-sized object may not be initialized", so this case should
not be possible.
Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1650-1652
+// If there is a list, but no init, it must be zero.
+if (i >= InitList->getNumInits())
+ return svalBuilder.makeZeroVal(R->getElementType());
r.stahl wrote:
> NoQ wrote:
> > NoQ wrote:
> > > Would this work correctly if the element is not of an integral or
> > > enumeration type? I think this needs an explicit check.
> > What if we have an out-of-bounds access to a variable-length array? I don't
> > think it'd yield zero.
> I'm getting "variable-sized object may not be initialized", so this case
> should not be possible.
I'm having a hard time reproducing this either.
```
struct S {
int a = 3;
};
S const sarr[2] = {};
void definit() {
int i = 1;
clang_analyzer_dump(sarr[i].a); // expected-warning{{test}}
}
```
results in a symbolic value, because makeZeroVal returns an empty SVal list for
arrays, records, vectors and complex types. Otherwise it just returns
UnknownVal (for example for not-yet-implemented floats).
Can you think of a case where this would be an issue?
https://reviews.llvm.org/D46823
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46001: [CodeComplete] Expose helpers to get RawComment of completion result.
ilya-biryukov updated this revision to Diff 146812.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.
- Rebase onto head and remove \brief from the comments, it's gone upstream now
- Rename CurrentArg to ArgIndex
Repository:
rC Clang
https://reviews.llvm.org/D46001
Files:
include/clang/Sema/CodeCompleteConsumer.h
lib/Sema/SemaCodeComplete.cpp
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -2765,27 +2765,11 @@
if (Declaration) {
Result.addParentContext(Declaration->getDeclContext());
Pattern->ParentName = Result.getParentName();
- // Provide code completion comment for self.GetterName where
- // GetterName is the getter method for a property with name
- // different from the property name (declared via a property
- // getter attribute.
- const NamedDecl *ND = Declaration;
- if (const ObjCMethodDecl *M = dyn_cast(ND))
-if (M->isPropertyAccessor())
- if (const ObjCPropertyDecl *PDecl = M->findPropertyDecl())
-if (PDecl->getGetterName() == M->getSelector() &&
-PDecl->getIdentifier() != M->getIdentifier()) {
- if (const RawComment *RC =
-Ctx.getRawCommentForAnyRedecl(M)) {
-Result.addBriefComment(RC->getBriefText(Ctx));
-Pattern->BriefComment = Result.getBriefComment();
- }
- else if (const RawComment *RC =
- Ctx.getRawCommentForAnyRedecl(PDecl)) {
-Result.addBriefComment(RC->getBriefText(Ctx));
-Pattern->BriefComment = Result.getBriefComment();
- }
-}
+ if (const RawComment *RC =
+ getPatternCompletionComment(Ctx, Declaration)) {
+Result.addBriefComment(RC->getBriefText(Ctx));
+Pattern->BriefComment = Result.getBriefComment();
+ }
}
return Pattern;
@@ -2845,14 +2829,9 @@
if (IncludeBriefComments) {
// Add documentation comment, if it exists.
-if (const RawComment *RC = Ctx.getRawCommentForAnyRedecl(ND)) {
+if (const RawComment *RC = getCompletionComment(Ctx, Declaration)) {
Result.addBriefComment(RC->getBriefText(Ctx));
}
-else if (const ObjCMethodDecl *OMD = dyn_cast(ND))
- if (OMD->isPropertyAccessor())
-if (const ObjCPropertyDecl *PDecl = OMD->findPropertyDecl())
- if (const RawComment *RC = Ctx.getRawCommentForAnyRedecl(PDecl))
-Result.addBriefComment(RC->getBriefText(Ctx));
}
if (StartsNestedNameSpecifier) {
@@ -3042,6 +3021,59 @@
return Result.TakeString();
}
+const RawComment *clang::getCompletionComment(const ASTContext &Ctx,
+ const NamedDecl *ND) {
+ if (!ND)
+return nullptr;
+ if (auto *RC = Ctx.getRawCommentForAnyRedecl(ND))
+return RC;
+
+ // Try to find comment from a property for ObjC methods.
+ const ObjCMethodDecl *M = dyn_cast(ND);
+ if (!M)
+return nullptr;
+ const ObjCPropertyDecl *PDecl = M->findPropertyDecl();
+ if (!PDecl)
+return nullptr;
+
+ return Ctx.getRawCommentForAnyRedecl(PDecl);
+}
+
+const RawComment *clang::getPatternCompletionComment(const ASTContext &Ctx,
+ const NamedDecl *ND) {
+ const ObjCMethodDecl *M = dyn_cast_or_null(ND);
+ if (!M || !M->isPropertyAccessor())
+return nullptr;
+
+ // Provide code completion comment for self.GetterName where
+ // GetterName is the getter method for a property with name
+ // different from the property name (declared via a property
+ // getter attribute.
+ const ObjCPropertyDecl *PDecl = M->findPropertyDecl();
+ if (!PDecl)
+return nullptr;
+ if (PDecl->getGetterName() == M->getSelector() &&
+ PDecl->getIdentifier() != M->getIdentifier()) {
+if (auto *RC = Ctx.getRawCommentForAnyRedecl(M))
+ return RC;
+if (auto *RC = Ctx.getRawCommentForAnyRedecl(PDecl))
+ return RC;
+ }
+ return nullptr;
+}
+
+const RawComment *clang::getParameterComment(
+const ASTContext &Ctx,
+const CodeCompleteConsumer::OverloadCandidate &Result,
+unsigned ArgIndex) {
+ auto FDecl = Result.getFunction();
+ if (!FDecl)
+return nullptr;
+ if (ArgIndex < FDecl->getNumParams())
+return Ctx.getRawCommentForAnyRedecl(FDecl->getParamDecl(ArgIndex));
+ return nullptr;
+}
+
/// Add function overload parameter chunks to the given code completion
/// string.
static void AddOverloadParameterChunks(ASTContext &Context,
@@ -3137,10 +3169,10 @@
}
if (FDecl) {
-if (IncludeBriefComments && CurrentArg < FDecl->getNumParams())
- if (auto RC = S.getASTContext().getRawCommentForAnyRedecl(
- FDecl->getParamDecl(CurrentArg)))
+if (IncludeBriefComments) {
+ if (auto RC = getParameterComment(S.getASTCont
[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.
MTC added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:1037
+
+if (StateWholeReg && !StateNotWholeReg && CharVal.isZeroConstant()) {
+ // If the 'memset()' acts on the whole region of destination buffer and
NoQ wrote:
> I think we should use `StateNonNullChar` (that's computed below) instead of
> `CharVal.isZeroConstant()` because the Environment doesn't provide a
> guarantee that all symbols within it are collapsed to constants where
> applicable.
Yea, thanks!
Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:1055
+std::tie(StateNullChar, StateNonNullChar) =
+assumeZero(C, State, CharVal, Ctx.UnsignedCharTy);
+
NoQ wrote:
> I think this should use `IntTy` here. Because that's the type of the
> `memset`'s argument, and that's what `assumeZero()` expects.
I confirmed again that `memset()` will convert the value to `unsigned char`
first, see http://en.cppreference.com/w/c/string/byte/memset.
In the next update, I will `evalCast(value, UnsignedCharTy, IntTy)` first,
therefore, I will continue to use `UnsignedCharTy`.
Comment at: test/Analysis/string.c:1412
+ clang_analyzer_eval(strlen(str) >= 10); // expected-warning{{TRUE}}
+ // FIXME: This shoule be TRUE.
+ clang_analyzer_eval(str[1] == '0'); // expected-warning{{UNKNOWN}}
NoQ wrote:
> Typo: `should` :)
thanks, will do!
Repository:
rC Clang
https://reviews.llvm.org/D44934
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r332314 - [AST] Fix printing tag decl groups in decl contexts
Hi Hans,
Thanks. Sorry for the trouble. I'll look into it.
Joel
On Tue, May 15, 2018 at 6:23 AM, Hans Wennborg wrote:
> This broke the ast-print-record-decl.c test on Windows, see for
> example http://lab.llvm.org:8011/builders/clang-with-thin-lto-
> windows/builds/9066
>
> One way to reproduce the failure on Linux is to pass a Windows triple
> to this ast-print command:
>
> --- a/test/Misc/ast-print-record-decl.c
> +++ b/test/Misc/ast-print-record-decl.c
> @@ -54,7 +54,7 @@
> // RUN:-DKW=struct -DBASES=' : B' -o - -xc++ %s \
> // RUN: | FileCheck --check-prefixes=CHECK,LLVM %s
> //
> -// RUN: %clang_cc1 -verify -ast-print -DKW=struct -DBASES=' : B' -xc++
> %s \
> +// RUN: %clang_cc1 -verify -triple i686-pc-win32 -ast-print
> -DKW=struct -DBASES=' : B' -xc++ %s \
> // RUN: > %t.cpp
> // RUN: FileCheck --check-prefixes=CHECK,PRINT,PRINT-CXX -DKW=struct \
> // RUN: -DBASES=' : B' %s --input-file %t.cpp
>
> What's happening is that on Windows, "__single_inheritance(1)" is
> printed before T1. But if I change the test to allow that in the
> output, it still doesn't pass as Clang doesn't seem able to parse it.
>
> I've worked around the problem by adding a Linux triple here in
> r332335, but someone should probably look into it properly.
>
> Thanks,
> Hans
>
> On Tue, May 15, 2018 at 2:44 AM, Joel E. Denny via cfe-commits
> wrote:
> > Author: jdenny
> > Date: Mon May 14 17:44:14 2018
> > New Revision: 332314
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=332314&view=rev
> > Log:
> > [AST] Fix printing tag decl groups in decl contexts
> >
> > For example, given:
> >
> > struct T1 {
> > struct T2 *p0;
> > };
> >
> > -ast-print produced:
> >
> > struct T1 {
> > struct T2;
> > struct T2 *p0;
> > };
> >
> > Compiling that produces a warning that the first struct T2 declaration
> > does not declare anything.
> >
> > Details:
> >
> > A tag decl group is one or more decls that share a type specifier that
> > is a tag decl (that is, a struct/union/class/enum decl). Within
> > functions, the parser builds such a tag decl group as part of a
> > DeclStmt. However, in decl contexts, such as file scope or a member
> > list, the parser does not group together the members of a tag decl
> > group. Previously, detection of tag decl groups during printing was
> > implemented but only if the tag decl was unnamed. Otherwise, as in
> > the above example, the members of the group did not print together and
> > so sometimes introduced warnings.
> >
> > This patch extends detection of tag decl groups in decl contexts to
> > any tag decl that is recorded in the AST as not free-standing.
> >
> > Reviewed by: rsmith
> >
> > Differential Revision: https://reviews.llvm.org/D45465
> >
> > Modified:
> > cfe/trunk/lib/AST/DeclPrinter.cpp
> > cfe/trunk/test/Misc/ast-print-enum-decl.c
> > cfe/trunk/test/Misc/ast-print-record-decl.c
> > cfe/trunk/test/Sema/ast-print.c
> > cfe/trunk/test/SemaCXX/ast-print.cpp
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46836: Fix some rtti-options tests
filcab accepted this revision. filcab added a comment. This revision is now accepted and ready to land. LGTM Comment at: test/Driver/rtti-options.cpp:13 // RUN: %clang -### -c -fno-rtti -frtti %s 2>&1 | FileCheck -check-prefix=CHECK-RTTI %s -// RUN: %clang -### -c -frtti -fno-rtti %s 2>&1 | FileCheck -check-prefix=CHECK-NO-RTTI %s +// RUN: %clang -### -c -frtti -fno-rtti %s 2>&1 | FileCheck -check-prefix=CHECK-RTTI-NOT %s Minor nit: I like saving space like that, but I think it might be slightly confusing for someone reading this, as the `-NOT` suffix is special in `FileCheck`. I don't think this is a huge problem as we have the single CHECK line, and we don't end up using `...-NOT-NOT` anyway. Feel free to keep or change as you want. Repository: rC Clang https://reviews.llvm.org/D46836 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.
MTC updated this revision to Diff 146816.
MTC added a comment.
- According to NoQ's suggestion, use `assumeZero()` instead of
`isZeroConstant()` to determine whether the value is 0.
- Add test `memset26_upper_UCHAR_MAX()` and `memset27_symbol()`
- Since `void *memset( void *dest, int ch, size_t count );` will converts the
value `ch` to `unsigned char`, we call `evalCast()` accordingly.
Repository:
rC Clang
https://reviews.llvm.org/D44934
Files:
lib/StaticAnalyzer/Checkers/CStringChecker.cpp
test/Analysis/bstring.cpp
test/Analysis/null-deref-ps-region.c
test/Analysis/string.c
Index: test/Analysis/string.c
===
--- test/Analysis/string.c
+++ test/Analysis/string.c
@@ -1,7 +1,8 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.cstring,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -Wno-null-dereference -verify %s
-// RUN: %clang_analyze_cc1 -DUSE_BUILTINS -analyzer-checker=core,unix.cstring,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -Wno-null-dereference -verify %s
-// RUN: %clang_analyze_cc1 -DVARIANT -analyzer-checker=core,unix.cstring,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -Wno-null-dereference -verify %s
-// RUN: %clang_analyze_cc1 -DUSE_BUILTINS -DVARIANT -analyzer-checker=alpha.security.taint,core,unix.cstring,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -Wno-null-dereference -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.cstring,unix.Malloc,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -Wno-null-dereference -verify %s
+// RUN: %clang_analyze_cc1 -DUSE_BUILTINS -analyzer-checker=core,unix.cstring,unix.Malloc,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -Wno-null-dereference -verify %s
+// RUN: %clang_analyze_cc1 -DVARIANT -analyzer-checker=core,unix.cstring,unix.Malloc,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -Wno-null-dereference -verify %s
+// RUN: %clang_analyze_cc1 -DUSE_BUILTINS -DVARIANT -analyzer-checker=alpha.security.taint,core,unix.cstring,unix.Malloc,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -Wno-null-dereference -verify %s
+// RUN: %clang_analyze_cc1 -DSUPPRESS_OUT_OF_BOUND -analyzer-checker=core,unix.cstring,unix.Malloc,alpha.unix.cstring.BufferOverlap,alpha.unix.cstring.NotNullTerminated,debug.ExprInspection -analyzer-store=region -Wno-null-dereference -verify %s
//===--===
// Declarations
@@ -1159,6 +1160,206 @@
clang_analyzer_eval(str[1] == 'b'); // expected-warning{{UNKNOWN}}
}
+//===--===
+// memset()
+//===--===
+
+void *memset(void *dest, int ch, size_t count);
+
+void *malloc(size_t size);
+void free(void *);
+
+void memset1_char_array_null() {
+ char str[] = "abcd";
+ clang_analyzer_eval(strlen(str) == 4); // expected-warning{{TRUE}}
+ memset(str, '\0', 2);
+ clang_analyzer_eval(strlen(str) == 0); // expected-warning{{TRUE}}
+}
+
+void memset2_char_array_null() {
+ char str[] = "abcd";
+ clang_analyzer_eval(strlen(str) == 4); // expected-warning{{TRUE}}
+ memset(str, '\0', strlen(str) + 1);
+ clang_analyzer_eval(strlen(str) == 0); // expected-warning{{TRUE}}
+ clang_analyzer_eval(str[2] == 0); // expected-warning{{TRUE}}
+}
+
+void memset3_char_malloc_null() {
+ char *str = (char *)malloc(10 * sizeof(char));
+ memset(str + 1, '\0', 8);
+ clang_analyzer_eval(str[1] == 0); // expected-warning{{UNKNOWN}}
+ free(str);
+}
+
+void memset4_char_malloc_null() {
+ char *str = (char *)malloc(10 * sizeof(char));
+ //void *str = malloc(10 * sizeof(char));
+ memset(str, '\0', 10);
+ clang_analyzer_eval(str[1] == 0); // expected-warning{{TRUE}}
+ clang_analyzer_eval(strlen(str) == 0); // expected-warning{{TRUE}}
+ free(str);
+}
+
+#ifdef SUPPRESS_OUT_OF_BOUND
+void memset5_char_malloc_overflow_null() {
+ char *str = (char *)malloc(10 * sizeof(char));
+ memset(str, '\0', 12);
+ clang_analyzer_eval(str[1] == 0); // expected-warning{{UNKNOWN}}
+ free(str);
+}
+#endif
+
+void memset6_char_array_nonnull() {
+ char str[] = "abcd";
+ clang_analyzer_eval(strlen(str) == 4); // expected-warning{{TRUE}}
+ memset(str, '0', 2);
+ clang_analyzer_eval(str[0] == 'a');// expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(strlen(str) == 4); // expected-warning{{UNKNOWN}}
+}
+
+#ifdef SUPPRESS_OUT_OF_BOUND
+void memset8_char_array_nonnull() {
+ char str[5] = "abcd";
+ clang_analyzer_eval(strlen(str) == 4); // expected-warning{{TRUE}}
+ memset(str, '0', 10);
+ clang_analyzer_eval(str[0] != '0'); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(strlen(str) >= 10); // expected-warning{{TRUE}}
+ clang_analyzer_eval(strlen(str) < 10); // expected-warning{{FALSE}}
+}
+#endif
+
+struct POD_memse
[PATCH] D46879: [clang-format] Fix putting ObjC message arguments in one line for multiline receiver
jolesiak created this revision.
Herald added subscribers: cfe-commits, klimek.
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
Repository:
rC Clang
https://reviews.llvm.org/D46879
Files:
lib/Format/ContinuationIndenter.cpp
unittests/Format/FormatTestObjC.cpp
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
@@ -1067,8 +1067,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 &CurrentScopeOpener =
+ *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
[PATCH] D46845: [libcxx][c++17] P0083R5: Splicing Maps and Sets
erik.pilkington added a comment. > One way we could deal with this is by adding an attribute to the compiler to > indicate "the const is a lie", that we can apply to `std::pair::first`, with > the semantics being that a top-level `const` is ignored when determining the > "real" type of the member (and so mutating the member after a `const_cast` > has defined behavior). This would apply to //all// `std::pair`s, not just > the `node_handle` case, but in practice we don't optimize on the basis of a > member being declared `const` *anyway*, so this isn't such a big deal right > now. I'm a fan of this idea, this would also have the bonus of fixing some pre-existing type-punning between `pair` and `pair` (see `__hash_value_type` and `__value_type` in `` and ``). > Another possibility would be a compiler intrinsic to change the type of the > pair, in-place, from a `std::pair` to a `std::pair`, > without changing anything about the members -- except that the `first` member > changes type. How could this work? It would still be possible for TBAA to assume that `void m(std::pair*, std::pair*);`'s parameters don't alias when optimizing `m`, regardless of how we form the non-const pair, right? > Yet another possibility would be to only ever access the `key` field through > a type annotated with `__attribute__((may_alias))`, and then launder the node > pointer when we're ready to insert it back into the container (to "pick up" > the new value of the const member). That's formally breaking the rules (a > `launder` isn't enough, because we didn't actually create a new object, and > in any case we still mutated the value of a `const` subobject), but it'll > probably work fine across compilers that support the attribute. I think we could fall back to this in cases where the attribute from idea (1) isn't available. There, we could mark std::pair as may_alias and still fix `__hash_value_type` and `__value_type`. This would pessimize std::pair, but only for "older" compilers. So unless I've gotten this all wrong, I'll write a patch to support that new attribute and come back to this patch if/when that lands. Sound good? Thanks for the feedback! Erik https://reviews.llvm.org/D46845 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46524: [clangd] Extract scoring/ranking logic, and shave yaks.
ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46524 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45999: [clangd] Retrieve minimally formatted comment text in completion.
ilya-biryukov updated this revision to Diff 146827.
ilya-biryukov marked 6 inline comments as done.
ilya-biryukov added a comment.
- Fix code after a change in deps (getFormattedText now needs a SourceManager
instead of an ASTContext)
- Address review comments
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D45999
Files:
clangd/CodeComplete.cpp
clangd/CodeComplete.h
clangd/CodeCompletionStrings.cpp
clangd/CodeCompletionStrings.h
clangd/index/SymbolCollector.cpp
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/CodeCompletionStringsTests.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -490,11 +490,11 @@
}
)";
runSymbolCollector(Header, /*Main=*/"");
- EXPECT_THAT(Symbols,
- UnorderedElementsAre(QName("nx"),
- AllOf(QName("nx::ff"),
- Labeled("ff(int x, double y)"),
- Detail("int"), Doc("Foo comment.";
+ EXPECT_THAT(
+ Symbols,
+ UnorderedElementsAre(
+ QName("nx"), AllOf(QName("nx::ff"), Labeled("ff(int x, double y)"),
+ Detail("int"), Doc("Foo comment.";
}
TEST_F(SymbolCollectorTest, PlainAndSnippet) {
Index: unittests/clangd/CodeCompletionStringsTests.cpp
===
--- unittests/clangd/CodeCompletionStringsTests.cpp
+++ unittests/clangd/CodeCompletionStringsTests.cpp
@@ -51,23 +51,24 @@
}
TEST_F(CompletionStringTest, Documentation) {
- Builder.addBriefComment("Is this brief?");
- EXPECT_EQ(getDocumentation(*Builder.TakeString()), "Is this brief?");
+ Builder.addBriefComment("This is ignored");
+ EXPECT_EQ(formatDocumentation(*Builder.TakeString(), "Is this brief?"),
+"Is this brief?");
}
TEST_F(CompletionStringTest, DocumentationWithAnnotation) {
- Builder.addBriefComment("Is this brief?");
+ Builder.addBriefComment("This is ignored");
Builder.AddAnnotation("Ano");
- EXPECT_EQ(getDocumentation(*Builder.TakeString()),
+ EXPECT_EQ(formatDocumentation(*Builder.TakeString(), "Is this brief?"),
"Annotation: Ano\n\nIs this brief?");
}
TEST_F(CompletionStringTest, MultipleAnnotations) {
Builder.AddAnnotation("Ano1");
Builder.AddAnnotation("Ano2");
Builder.AddAnnotation("Ano3");
- EXPECT_EQ(getDocumentation(*Builder.TakeString()),
+ EXPECT_EQ(formatDocumentation(*Builder.TakeString(), ""),
"Annotations: Ano1 Ano2 Ano3\n");
}
@@ -97,7 +98,7 @@
TEST_F(CompletionStringTest, FunctionSnippet) {
Builder.AddResultTypeChunk("result no no");
- Builder.addBriefComment("Foo's comment");
+ Builder.addBriefComment("This comment is ignored");
Builder.AddTypedTextChunk("Foo");
Builder.AddChunk(CodeCompletionString::CK_LeftParen);
Builder.AddPlaceholderChunk("p1");
@@ -113,7 +114,7 @@
labelAndInsertText(*CCS, /*EnableSnippets=*/true);
EXPECT_EQ(Label, "Foo(p1, p2)");
EXPECT_EQ(InsertText, "Foo(${1:p1}, ${2:p2})");
- EXPECT_EQ(getDocumentation(*CCS), "Foo's comment");
+ EXPECT_EQ(formatDocumentation(*CCS, "Foo's comment"), "Foo's comment");
EXPECT_EQ(getFilterText(*CCS), "Foo");
}
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -243,8 +243,7 @@
// completion. At least there aren't any we're aware of.
EXPECT_THAT(Results.items, Not(Contains(Kind(CompletionItemKind::Snippet;
// Check documentation.
- EXPECT_IFF(Opts.IncludeBriefComments, Results.items,
- Contains(IsDocumented()));
+ EXPECT_IFF(Opts.IncludeComments, Results.items, Contains(IsDocumented()));
}
void TestGlobalScopeCompletion(clangd::CodeCompleteOptions Opts) {
@@ -289,8 +288,7 @@
AllOf(Has("local_var"), Has("LocalClass"),
Contains(Kind(CompletionItemKind::Snippet;
// Check documentation.
- EXPECT_IFF(Opts.IncludeBriefComments, Results.items,
- Contains(IsDocumented()));
+ EXPECT_IFF(Opts.IncludeComments, Results.items, Contains(IsDocumented()));
}
TEST(CompletionTest, CompletionOptions) {
@@ -301,7 +299,7 @@
// We used to test every combination of options, but that got too slow (2^N).
auto Flags = {
&clangd::CodeCompleteOptions::IncludeMacros,
- &clangd::CodeCompleteOptions::IncludeBriefComments,
+ &clangd::CodeCompleteOptions::IncludeComments,
&clangd::CodeCompleteOptions::EnableSnippets,
&clangd::CodeCompleteOptions::IncludeCodePatterns,
&clangd::CodeCompleteOptions::IncludeIneligibleResults,
Index: clangd/index/SymbolCollector.cpp
[PATCH] D46241: [CodeGen] Recognize more cases of zero initialization
sepavloff added a comment. @rsmith Do yo think this patch is OK to apply? Repository: rC Clang https://reviews.llvm.org/D46241 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45999: [clangd] Retrieve minimally formatted comment text in completion.
ilya-biryukov added inline comments. Comment at: clangd/CodeCompletionStrings.h:29 + +/// Gets a raw documentation comment of the current active parameter +/// of \p Result. sammccall wrote: > sammccall wrote: > > ilya-biryukov wrote: > > > ilya-biryukov wrote: > > > > sammccall wrote: > > > > > sammccall wrote: > > > > > > "active" is a bit confusing - at this level we just care which arg > > > > > > you're interested in, right? > > > > > Is this typically a substring of getDocComment for the function? If > > > > > so, noting that would make this clearer. > > > > This refers to the parameter that should be reported as active in > > > > `signatureHelp`. > > > > Agree that it requires too much context to understand, changed to a > > > > more detailed description involving `CurrentArg` parameter. > > > > > > > > > > > > Is this typically a substring of getDocComment for the function? If so, > > > > noting that would make this clearer. > > > I mechanically translated the original source code without looking too > > > much on what it does :-) > > > > > > Extracting a substring from the function doc would be the behavior I > > > expect. > > > However, we simply try to get doc comment for parameter in the same way > > > we do for functions (e.g. look for comment doc for parameter decl, > > > without looking at function decl in the first place) > > > It means we'll always get the empty doc currently. At least, I couldn't > > > make the current code produce any docs for the parameter. > > > > > > I would still keep this function, as it seems to be called in the right > > > places. But we definitely need to parse a function comment instead. > > I think this function is easy to understand in isolation, and by naming the > > parameter `CurrentArg` one can only really understand it in terms of its > > callsite in `signatureHelp`. I'd name it `ArgIndex` or similar instead, but > > up to you. > Ah, thanks. > I'd suggest noting that in the comment (this currently looks for comments > attached to the parameter itself, and doesn't extract them from function > documentation). I think most people looking at this would be interested in > that detail! > > Looking at the doxygen docs, it seems like they want syntax like this to > work:`int foo(int x /** Doc */)`, and I'd hope we could also get this to work > at some point: > ```int foo( > // Doc > int x > );``` > > Not that I see much code in that style, but maybe we should write more! Noting that is definitely useful. And yes, ideally we should support both styles. Comment at: unittests/clangd/CodeCompleteTests.cpp:246 // Check documentation. - EXPECT_IFF(Opts.IncludeBriefComments, Results.items, - Contains(IsDocumented())); sammccall wrote: > You've updated the existing doxygen test cases, but I don't think you're > testing the new non-doxygen functionality anywhere? Yep, I moved all the tests to clang when reviewing the patch. I'll add more tests to the last change that actually sets ParseAllComments to true. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45999 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46751: [clangd] Filter out private proto symbols in SymbolCollector.
malaperle added a comment. In https://reviews.llvm.org/D46751#1099097, @ioeric wrote: > > I think this is good if that's true that the comment is always there. I > > think it would be OK for this to be enabled by default, with a general > > option to turn heuristics off. Not sure what to call it... > > -use-symbol-filtering-heuristics :) > > We have other filters in the symbol collector that we think could improve > user experience, and we don't provide options for those. What others filters do you mean? If you mean skipping "members", symbols in main files, etc, I a working on making them not skipped, see https://reviews.llvm.org/D44954. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46751 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46881: [X86][CET] Changing -fcf-protection behavior to comply with gcc (clang part)
mike.dvoretsky created this revision. mike.dvoretsky added a reviewer: craig.topper. Herald added a subscriber: cfe-commits. This patch aims to match the changes introduced in gcc by https://gcc.gnu.org/ml/gcc-cvs/2018-04/msg00534.html. The -mibt feature flag is being removed, and the -fcf-protection option now also defines a __CET__ macro and causes errors when used on non-X86 targets, while X86 targets no longer check for -mibt and -mshstk to determine if -fcf-protection is supported. -mshstk is now used only to determine availability of shadow stack intrinsics. Comes with an LLVM patch. Repository: rC Clang https://reviews.llvm.org/D46881 Files: clang/docs/ClangCommandLineReference.rst clang/include/clang/Basic/DiagnosticCommonKinds.td clang/include/clang/Basic/TargetInfo.h clang/include/clang/Driver/Options.td clang/lib/Basic/TargetInfo.cpp clang/lib/Basic/Targets/X86.cpp clang/lib/Basic/Targets/X86.h clang/lib/Frontend/CompilerInvocation.cpp clang/test/CodeGen/attributes.c clang/test/CodeGen/builtins-x86.c clang/test/CodeGen/x86-cf-protection.c clang/test/Driver/x86-target-features.c clang/test/Preprocessor/x86_target_features.c clang/test/Sema/attr-nocf_check.c clang/test/Sema/attr-nocf_check.cpp Index: clang/test/Sema/attr-nocf_check.cpp === --- clang/test/Sema/attr-nocf_check.cpp +++ clang/test/Sema/attr-nocf_check.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -triple=i386-unknown-unknown -verify -fcf-protection=branch -target-feature +ibt -std=c++11 -fsyntax-only %s +// RUN: %clang_cc1 -triple=i386-unknown-unknown -verify -fcf-protection=branch -std=c++11 -fsyntax-only %s // Function pointer definition. [[gnu::nocf_check]] typedef void (*FuncPointerWithNoCfCheck)(void); // no-warning Index: clang/test/Sema/attr-nocf_check.c === --- clang/test/Sema/attr-nocf_check.c +++ clang/test/Sema/attr-nocf_check.c @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -triple=x86_64-unknown-unknown -verify -fcf-protection=branch -target-feature +ibt -fsyntax-only %s +// RUN: %clang_cc1 -triple=x86_64-unknown-unknown -verify -fcf-protection=branch -fsyntax-only %s // Function pointer definition. typedef void (*FuncPointerWithNoCfCheck)(void) __attribute__((nocf_check)); // no-warning Index: clang/test/Preprocessor/x86_target_features.c === --- clang/test/Preprocessor/x86_target_features.c +++ clang/test/Preprocessor/x86_target_features.c @@ -380,10 +380,6 @@ // SHSTK: #define __SHSTK__ 1 -// RUN: %clang -target i386-unknown-unknown -mibt -x c -E -dM -o - %s | FileCheck -match-full-lines --check-prefix=IBT %s - -// IBT: #define __IBT__ 1 - // RUN: %clang -target i386-unknown-unknown -march=atom -mrdseed -x c -E -dM -o - %s | FileCheck -match-full-lines --check-prefix=RDSEED %s // RDSEED: #define __RDSEED__ 1 Index: clang/test/Driver/x86-target-features.c === --- clang/test/Driver/x86-target-features.c +++ clang/test/Driver/x86-target-features.c @@ -80,11 +80,6 @@ // CETSS: "-target-feature" "+shstk" // NO-CETSS: "-target-feature" "-shstk" -// RUN: %clang -target i386-unknown-linux-gnu -march=i386 -mibt %s -### -o %t.o 2>&1 | FileCheck -check-prefix=CETIBT %s -// RUN: %clang -target i386-unknown-linux-gnu -march=i386 -mno-ibt %s -### -o %t.o 2>&1 | FileCheck -check-prefix=NO-CETIBT %s -// CETIBT: "-target-feature" "+ibt" -// NO-CETIBT: "-target-feature" "-ibt" - // RUN: %clang -target i386-unknown-linux-gnu -march=i386 -msgx %s -### -o %t.o 2>&1 | FileCheck -check-prefix=SGX %s // RUN: %clang -target i386-unknown-linux-gnu -march=i386 -mno-sgx %s -### -o %t.o 2>&1 | FileCheck -check-prefix=NO-SGX %s // SGX: "-target-feature" "+sgx" Index: clang/test/CodeGen/x86-cf-protection.c === --- clang/test/CodeGen/x86-cf-protection.c +++ clang/test/CodeGen/x86-cf-protection.c @@ -1,6 +1,8 @@ -// RUN: not %clang_cc1 -fsyntax-only -S -o %t -triple i386-unknown-unknown -fcf-protection=return %s 2>&1 | FileCheck %s --check-prefix=RETURN -// RUN: not %clang_cc1 -fsyntax-only -S -o %t -triple i386-unknown-unknown -fcf-protection=branch %s 2>&1 | FileCheck %s --check-prefix=BRANCH +// RUN: %clang -target i386-unknown-unknown -x c -E -dM -o - -fcf-protection=return %s | FileCheck %s --check-prefix=RETURN +// RUN: %clang -target i386-unknown-unknown -x c -E -dM -o - -fcf-protection=branch %s | FileCheck %s --check-prefix=BRANCH +// RUN: %clang -target i386-unknown-unknown -x c -E -dM -o - -fcf-protection=full %s | FileCheck %s --check-prefix=FULL -// RETURN: error: option 'cf-protection=return' cannot be specified without '-mshstk' -// BRANCH: error: option 'cf-protection=branch' cannot be specified without '-mibt' +// RETURN: #define __CET__ 2 +// BRANCH: #define __CET__
[PATCH] D46751: [clangd] Filter out private proto symbols in SymbolCollector.
ioeric added a comment. In https://reviews.llvm.org/D46751#1099479, @malaperle wrote: > In https://reviews.llvm.org/D46751#1099097, @ioeric wrote: > > > > I think this is good if that's true that the comment is always there. I > > > think it would be OK for this to be enabled by default, with a general > > > option to turn heuristics off. Not sure what to call it... > > > -use-symbol-filtering-heuristics :) > > > > We have other filters in the symbol collector that we think could improve > > user experience, and we don't provide options for those. > > > What others filters do you mean? If you mean skipping "members", symbols in > main files, etc, I a working on making them not skipped, see > https://reviews.llvm.org/D44954. I meant the filters in https://github.com/llvm-mirror/clang-tools-extra/blob/master/clangd/index/SymbolCollector.cpp#L93 e.g. filtering symbols in anonymous namespace, which we think should never appear in the index. I think members are more interesting than the private proto symbols. We want to un-filter members because there are features that would use them, so indexing them makes sense. But private proto symbols should never be shown to users (e.g. in code completion or workspaceSymbols). I also think adding an option for indexing members would actually make more sense because they might significantly increase the index size, and it would be good to have options to disable it if users don't use members (e.g. including members can increase size of our internal global index service, and we are not sure if we are ready for that). Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46751 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46002: [clangd] Parse all comments in Sema and completion.
ilya-biryukov updated this revision to Diff 146831.
ilya-biryukov added a comment.
- Added tests that all comments are being parsed.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D46002
Files:
clangd/ClangdUnit.cpp
clangd/CodeComplete.cpp
unittests/clangd/CodeCompleteTests.cpp
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -574,6 +574,30 @@
Doc("Doooc"), Detail("void";
}
+TEST(CompletionTest, Documentation) {
+ auto Results = completions(
+ R"cpp(
+ // Non-doxygen comment.
+ int foo();
+ /// Doxygen comment.
+ /// \param int a
+ int bar(int a);
+ /* Multi-line
+ block comment
+ */
+ int baz();
+
+ int x = ^
+ )cpp");
+ EXPECT_THAT(Results.items,
+ Contains(AllOf(Named("foo"), Doc("Non-doxygen comment.";
+ EXPECT_THAT(
+ Results.items,
+ Contains(AllOf(Named("bar"), Doc("Doxygen comment.\n\\param int a";
+ EXPECT_THAT(Results.items,
+ Contains(AllOf(Named("baz"), Doc("Multi-line\nblock comment";
+}
+
TEST(CodeCompleteTest, DisableTypoCorrection) {
auto Results = completions(R"cpp(
namespace clang { int v; }
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -22,6 +22,7 @@
#include "SourceCode.h"
#include "Trace.h"
#include "index/Index.h"
+#include "clang/Basic/LangOptions.h"
#include "clang/Format/Format.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/FrontendActions.h"
@@ -706,6 +707,7 @@
return false;
}
CI->getFrontendOpts().DisableFree = false;
+ CI->getLangOpts()->CommentOpts.ParseAllComments = true;
std::unique_ptr ContentsBuffer =
llvm::MemoryBuffer::getMemBufferCopy(Input.Contents, Input.FileName);
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -25,6 +25,7 @@
#include "clang/Sema/Sema.h"
#include "clang/Serialization/ASTWriter.h"
#include "clang/Tooling/CompilationDatabase.h"
+#include "clang/Basic/LangOptions.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/CrashRecoveryContext.h"
@@ -346,6 +347,7 @@
}
// createInvocationFromCommandLine sets DisableFree.
CI->getFrontendOpts().DisableFree = false;
+CI->getLangOpts()->CommentOpts.ParseAllComments = true;
}
std::unique_ptr ContentsBuffer =
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -574,6 +574,30 @@
Doc("Doooc"), Detail("void";
}
+TEST(CompletionTest, Documentation) {
+ auto Results = completions(
+ R"cpp(
+ // Non-doxygen comment.
+ int foo();
+ /// Doxygen comment.
+ /// \param int a
+ int bar(int a);
+ /* Multi-line
+ block comment
+ */
+ int baz();
+
+ int x = ^
+ )cpp");
+ EXPECT_THAT(Results.items,
+ Contains(AllOf(Named("foo"), Doc("Non-doxygen comment.";
+ EXPECT_THAT(
+ Results.items,
+ Contains(AllOf(Named("bar"), Doc("Doxygen comment.\n\\param int a";
+ EXPECT_THAT(Results.items,
+ Contains(AllOf(Named("baz"), Doc("Multi-line\nblock comment";
+}
+
TEST(CodeCompleteTest, DisableTypoCorrection) {
auto Results = completions(R"cpp(
namespace clang { int v; }
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -22,6 +22,7 @@
#include "SourceCode.h"
#include "Trace.h"
#include "index/Index.h"
+#include "clang/Basic/LangOptions.h"
#include "clang/Format/Format.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/FrontendActions.h"
@@ -706,6 +707,7 @@
return false;
}
CI->getFrontendOpts().DisableFree = false;
+ CI->getLangOpts()->CommentOpts.ParseAllComments = true;
std::unique_ptr ContentsBuffer =
llvm::MemoryBuffer::getMemBufferCopy(Input.Contents, Input.FileName);
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -25,6 +25,7 @@
#include "clang/Sema/Sema.h"
#include "clang/Serialization/ASTWriter.h"
#include "clang/Tooling/CompilationDatabase.h"
+#include "clang/Basic/LangOptions.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/CrashRecoveryContext.h"
@@ -346,6 +347,7 @@
}
// createInvocationFromCommandLine sets Dis
[PATCH] D45900: CodeGen: Fix invalid bitcast for lifetime.start/end
yaxunl added a comment.
In https://reviews.llvm.org/D45900#1093160, @rjmccall wrote:
> In https://reviews.llvm.org/D45900#1093154, @yaxunl wrote:
>
> > In https://reviews.llvm.org/D45900#1083377, @rjmccall wrote:
> >
> > > Oh, I see, it's not that the lifetime intrinsics don't handle pointers in
> > > the alloca address space, it's that we might have already promoted them
> > > into `DefaultAS`.
> > >
> > > Do the LLVM uses of lifetime intrinsics actually look through these
> > > address space casts? I'm wondering if we might need to change how we
> > > emit the intrinsics so that they're emitted directly on (bitcasts of) the
> > > underlying allocas.
> >
> >
> > Some passes do not look through address space casts. Although there is
> > InferAddressSpace pass which can eliminate the redundant address space
> > casts, still it is desirable not to emit redundant address space in Clang.
> >
> > To avoid increasing complexity of alloca emitting API, I think we need a
> > way to track the original alloca and the alloca casted to default address
> > space. I can think of two ways:
> >
> > 1. add OriginalPointer member to Address, which is the originally emitted
> > LLVM value for the variable. Whenever we pass the address of a variable we
> > also pass the original LLVM value.
> > 2. add a map to CodeGenFunction to map the casted alloca to the real alloca.
> >
> > Any suggestion? Thanks.
>
>
> Can we just call CreateLifetimeStart (and push the cleanup to call
> CreateLifetimeEnd) immediately after creating the alloca instead of waiting
> until later like we do now?
>
> Modifying Address is not appropriate, and adding a map to CGF would be big
> waste.
Since CreateTempAlloca returns the casted alloca by default, whereas
CreateLifetimeStart expects the original alloca. If we want to call
CreateLifetimeStart with original alloca, we have to do it in CreateTempAlloca.
This incurs two issues:
1. we need an enum parameter to control how lifetime.start/end is emitted.
There are cases that lifetime.start is not emitted, and different ways to push
cleanups for lifetime.end, e.g. in CodeGenFunction::
EmitMaterializeTemporaryExpr
switch (M->getStorageDuration()) {
case SD_Automatic:
case SD_FullExpression:
if (auto *Size = EmitLifetimeStart(
CGM.getDataLayout().getTypeAllocSize(Object.getElementType()),
Object.getPointer())) {
if (M->getStorageDuration() == SD_Automatic)
pushCleanupAfterFullExpr(NormalEHLifetimeMarker,
Object, Size);
else
pushFullExprCleanup(NormalEHLifetimeMarker, Object,
Size);
}
break;
2. There are situations that the pushed cleanup for lifetime.end is deactivated
and emitted early, e.g. in AggExprEmitter::withReturnValueSlot
// If there's no dtor to run, the copy was the last use of our temporary.
// Since we're not guaranteed to be in an ExprWithCleanups, clean up
// eagerly.
CGF.DeactivateCleanupBlock(LifetimeEndBlock, LifetimeStartInst);
CGF.EmitLifetimeEnd(LifetimeSizePtr, RetAddr.getPointer());
In this case there is no good way to get the LifetimeStartInst and the original
alloca inst.
Basically the emitting of lifetime.start/end is not uniform enough to be
incorporated as part of CreateTempAlloca.
How about letting CreateTempAlloca have an optional pointer argument for
returning the original alloca?
https://reviews.llvm.org/D45900
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46871: [AMDGPU] Add interpolation builtins
arsenm added inline comments. Comment at: test/CodeGenOpenCL/builtins-amdgcn-interp.cl:2-28 +// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu gfx900 -S -o - %s | FileCheck %s --check-prefixes=CHECK,GFX9,BANK32 +// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu fiji -S -o - %s | FileCheck %s --check-prefixes=CHECK,GFX8,BANK32 +// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu gfx810 -S -o - %s | FileCheck %s --check-prefixes=CHECK,GFX8,BANK16 + +#pragma OPENCL EXTENSION cl_khr_fp16 : enable + +// CHECK-LABEL: test_interp_f16 These should be emitting / checking the IR, not the output asm Repository: rC Clang https://reviews.llvm.org/D46871 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46871: [AMDGPU] Add interpolation builtins
arsenm added inline comments. Comment at: include/clang/Basic/BuiltinsAMDGPU.def:103-107 +BUILTIN(__builtin_amdgcn_interp_p1_f16, "ffUiUibUi", "nc") +BUILTIN(__builtin_amdgcn_interp_p2_f16, "hffUiUibUi", "nc") +BUILTIN(__builtin_amdgcn_interp_p1, "ffUiUiUi", "nc") +BUILTIN(__builtin_amdgcn_interp_p2, "fffUiUiUi", "nc") +BUILTIN(__builtin_amdgcn_interp_mov, "fUiUiUiUi", "nc") You will also need C++ handling for these when the intrinsics are fixed to use name mangling Repository: rC Clang https://reviews.llvm.org/D46871 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46871: [AMDGPU] Add interpolation builtins
timcorringham added inline comments. Comment at: include/clang/Basic/BuiltinsAMDGPU.def:103-107 +BUILTIN(__builtin_amdgcn_interp_p1_f16, "ffUiUibUi", "nc") +BUILTIN(__builtin_amdgcn_interp_p2_f16, "hffUiUibUi", "nc") +BUILTIN(__builtin_amdgcn_interp_p1, "ffUiUiUi", "nc") +BUILTIN(__builtin_amdgcn_interp_p2, "fffUiUiUi", "nc") +BUILTIN(__builtin_amdgcn_interp_mov, "fUiUiUiUi", "nc") arsenm wrote: > You will also need C++ handling for these when the intrinsics are fixed to > use name mangling Can you expand on that please, I don't know what needs to be done here? Comment at: test/CodeGenOpenCL/builtins-amdgcn-interp.cl:2-28 +// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu gfx900 -S -o - %s | FileCheck %s --check-prefixes=CHECK,GFX9,BANK32 +// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu fiji -S -o - %s | FileCheck %s --check-prefixes=CHECK,GFX8,BANK32 +// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu gfx810 -S -o - %s | FileCheck %s --check-prefixes=CHECK,GFX8,BANK16 + +#pragma OPENCL EXTENSION cl_khr_fp16 : enable + +// CHECK-LABEL: test_interp_f16 arsenm wrote: > These should be emitting / checking the IR, not the output asm Checking the IR doesn't allow you to check the behavior very well, as there are differences depending on the target-cpu which are not apparent in the IR. Repository: rC Clang https://reviews.llvm.org/D46871 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call
Szelethus added a comment. I'm afraid it'll be even more time before I post a new diff. There are some things that `ProgramState` is lacking, so I'll get that fixed first in a different pull request first. Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:258-260 +Report->addNote(NoteBuf, +PathDiagnosticLocation::create(FieldChain.getEndOfChain(), + Context.getSourceManager())); NoQ wrote: > NoQ wrote: > > Szelethus wrote: > > > NoQ wrote: > > > > Aha, ok, got it, so you're putting the warning at the end of the > > > > constructor and squeezing all uninitialized fields into a single > > > > warning by presenting them as notes. > > > > > > > > Because for now notes aren't supported everywhere (and aren't always > > > > supported really well), i guess it'd be necessary to at least provide > > > > an option to make note-less reports before the checker is enabled by > > > > default everywhere. In this case notes contain critical pieces of > > > > information and as such cannot be really discarded. > > > > > > > > I'm not sure that notes are providing a better user experience here > > > > than simply saying that "field x.y->z is never initialized". With > > > > notes, the user will have to scroll around (at least in scan-build) to > > > > find which field is uninitialized. > > > > > > > > Notes will probably also not decrease the number of reports too much > > > > because in most cases there will be just one uninitialized field. > > > > Though i agree that when there is more than one report, the user will > > > > be likely to deal with all fields at once rather than separately. > > > > > > > > So it's a bit wonky. > > > > > > > > We might want to enable one mode or another in the checker depending on > > > > the analyzer output flag. Probably in the driver > > > > (`RenderAnalyzerOptions()`). > > > > > > > > It'd be a good idea to land the checker into alpha first and fix this > > > > in a follow-up patch because this review is already overweight. > > > > [...]i guess it'd be necessary to at least provide an option to make > > > > note-less reports before the checker is enabled by default everywhere. > > > > In this case notes contain critical pieces of information and as such > > > > cannot be really discarded. > > > This can already be achieved with `-analyzer-config > > > notes-as-events=true`. There no good reason however not to make an > > > additional flag that would emit warnings instead of notes. > > > > I'm not sure that notes are providing a better user experience here > > > > than simply saying that "field x.y->z is never initialized". With > > > > notes, the user will have to scroll around (at least in scan-build) to > > > > find which field is uninitialized. > > > This checker had a previous implementation that emitted a warning for > > > each uninitialized field, instead of squeezing them in a single warning > > > per constructor call. One of the major reasons behind rewriting it was > > > that it emitted so many of them that it was difficult to differentiate > > > which warning belonged to which constructor call. Also, in the case of > > > the LLVM/Clang project, the warnings from this checker alone would be in > > > the thousands. > > > > Notes will probably also not decrease the number of reports too much > > > > because in most cases there will be just one uninitialized field. > > > While this is true to some extent, it's not uncommon that a single > > > constructor call leaves 7 uninitialized -- in the LLVM/Clang project I > > > found one that had over 30! > > > Since its practically impossible to determine whether this was a > > > performance enhancement or just poor programming, the few cases where it > > > is intentional, an object would emit many more warnings would make make > > > majority of the findings (where an object truly had only 1 or 2 fields > > > uninit) a lot harder to spot in my opinion. > > > > > > In general, I think one warning per constructor call makes the best user > > > experience, but a temporary solution should be implemented until there's > > > better support for notes. > > > > > > I agree, this should be a topic of a follow-up patch. > > > > > > This can already be achieved with `-analyzer-config notes-as-events=true`. > > > > Yep. But the resulting user experience seems pretty bad to me. It was a > > good quick proof-of-concept trick to test things, but the fact that notes > > aren't path pieces is way too apparent in case of this checker. So i guess > > this flag was a bad idea. > > > > > Also, in the case of the LLVM/Clang project, the warnings from this > > > checker alone would be in the thousands. > > > > This makes me a bit worried, i wonder what's the reason why does the > > checker shout so loudly on a codebase that doesn't seem to have actual > > uninitialized value
[PATCH] D46497: [clangd] Populate #include insertions as additional edits in completion items.
ioeric updated this revision to Diff 146834.
ioeric marked 4 inline comments as done.
ioeric added a comment.
- Addressed review comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D46497
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/CodeComplete.cpp
clangd/CodeComplete.h
clangd/Headers.cpp
clangd/Headers.h
clangd/Protocol.cpp
clangd/Protocol.h
test/clangd/initialize-params-invalid.test
test/clangd/initialize-params.test
test/clangd/insert-include.test
unittests/clangd/ClangdTests.cpp
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/HeadersTests.cpp
Index: unittests/clangd/HeadersTests.cpp
===
--- unittests/clangd/HeadersTests.cpp
+++ unittests/clangd/HeadersTests.cpp
@@ -8,38 +8,97 @@
//===--===//
#include "Headers.h"
+
+#include "Compiler.h"
#include "TestFS.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Lex/PreprocessorOptions.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
namespace clang {
namespace clangd {
namespace {
+using ::testing::AllOf;
+using ::testing::UnorderedElementsAre;
+
class HeadersTest : public ::testing::Test {
public:
HeadersTest() {
CDB.ExtraClangFlags = {SearchDirArg.c_str()};
FS.Files[MainFile] = "";
+// Make sure directory sub/ exists.
+FS.Files[testPath("sub/EMPTY")] = "";
+ }
+
+private:
+ std::unique_ptr setupClang() {
+auto Cmd = CDB.getCompileCommand(MainFile);
+assert(static_cast(Cmd));
+auto VFS = FS.getFileSystem();
+VFS->setCurrentWorkingDirectory(Cmd->Directory);
+
+std::vector Argv;
+for (const auto &S : Cmd->CommandLine)
+ Argv.push_back(S.c_str());
+auto CI = clang::createInvocationFromCommandLine(
+Argv,
+CompilerInstance::createDiagnostics(new DiagnosticOptions(),
+&IgnoreDiags, false),
+VFS);
+EXPECT_TRUE(static_cast(CI));
+CI->getFrontendOpts().DisableFree = false;
+
+// The diagnostic options must be set before creating a CompilerInstance.
+CI->getDiagnosticOpts().IgnoreWarnings = true;
+auto Clang = prepareCompilerInstance(
+std::move(CI), /*Preamble=*/nullptr,
+llvm::MemoryBuffer::getMemBuffer(FS.Files[MainFile], MainFile),
+std::make_shared(), VFS, IgnoreDiags);
+
+EXPECT_FALSE(Clang->getFrontendOpts().Inputs.empty());
+return Clang;
}
protected:
+ std::vector collectIncludes() {
+auto Clang = setupClang();
+PreprocessOnlyAction Action;
+EXPECT_TRUE(
+Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0]));
+std::vector Inclusions;
+Clang->getPreprocessor().addPPCallbacks(collectInclusionsInMainFileCallback(
+Clang->getSourceManager(),
+[&](Inclusion Inc) { Inclusions.push_back(std::move(Inc)); }));
+EXPECT_TRUE(Action.Execute());
+Action.EndSourceFile();
+return Inclusions;
+ }
+
// Calculates the include path, or returns "" on error.
std::string calculate(PathRef Original, PathRef Preferred = "",
+const std::vector &Inclusions = {},
bool ExpectError = false) {
+auto Clang = setupClang();
+PreprocessOnlyAction Action;
+EXPECT_TRUE(
+Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0]));
+
if (Preferred.empty())
Preferred = Original;
-auto VFS = FS.getFileSystem();
-auto Cmd = CDB.getCompileCommand(MainFile);
-assert(static_cast(Cmd));
-VFS->setCurrentWorkingDirectory(Cmd->Directory);
auto ToHeaderFile = [](llvm::StringRef Header) {
return HeaderFile{Header,
/*Verbatim=*/!llvm::sys::path::is_absolute(Header)};
};
-auto Path = calculateIncludePath(MainFile, FS.Files[MainFile],
- ToHeaderFile(Original),
- ToHeaderFile(Preferred), *Cmd, VFS);
+
+auto Path = calculateIncludePath(
+MainFile, CDB.getCompileCommand(MainFile)->Directory,
+Clang->getPreprocessor().getHeaderSearchInfo(), Inclusions,
+ToHeaderFile(Original), ToHeaderFile(Preferred));
+Action.EndSourceFile();
if (!Path) {
llvm::consumeError(Path.takeError());
EXPECT_TRUE(ExpectError);
@@ -49,52 +108,50 @@
}
return std::move(*Path);
}
+
+ Expected>
+ insert(const HeaderFile &DeclaringHeader, const HeaderFile &InsertedHeader,
+ const std::vector &ExistingInclusions = {}) {
+auto Clang = setupClang();
+PreprocessOnlyAction Action;
+EXPECT_TRUE(
+Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0]));
+
+IncludeInserter Inserter(MainFile, /*Code=*/"", format
[PATCH] D46700: [ThinLTO] Add testing of new summary index format to a couple CFI tests
tejohnson updated this revision to Diff 146836. tejohnson added a comment. Update tests for changes to https://reviews.llvm.org/D46699 Repository: rC Clang https://reviews.llvm.org/D46700 Files: test/CodeGen/thinlto-distributed-cfi-devirt.ll test/CodeGen/thinlto-distributed-cfi.ll Index: test/CodeGen/thinlto-distributed-cfi.ll === --- test/CodeGen/thinlto-distributed-cfi.ll +++ test/CodeGen/thinlto-distributed-cfi.ll @@ -20,6 +20,11 @@ ; CHECK: blob data = '_ZTS1A' ; CHECK-LABEL: Index: test/CodeGen/thinlto-distributed-cfi.ll === --- test/CodeGen/thinlto-distributed-cfi.ll +++ test/CodeGen/thinlto-distributed-cfi.ll @@ -20,6 +20,11 @@ ; CHECK: blob data = '_ZTS1A' ; CHECK-LABEL: ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46497: [clangd] Populate #include insertions as additional edits in completion items.
ioeric updated this revision to Diff 146838.
ioeric added a comment.
- Rebase on https://reviews.llvm.org/D46676
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D46497
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/CodeComplete.cpp
clangd/CodeComplete.h
clangd/Headers.cpp
clangd/Headers.h
clangd/Protocol.cpp
clangd/Protocol.h
test/clangd/initialize-params-invalid.test
test/clangd/initialize-params.test
test/clangd/insert-include.test
unittests/clangd/ClangdTests.cpp
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/HeadersTests.cpp
Index: unittests/clangd/HeadersTests.cpp
===
--- unittests/clangd/HeadersTests.cpp
+++ unittests/clangd/HeadersTests.cpp
@@ -8,38 +8,97 @@
//===--===//
#include "Headers.h"
+
+#include "Compiler.h"
#include "TestFS.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Lex/PreprocessorOptions.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
namespace clang {
namespace clangd {
namespace {
+using ::testing::AllOf;
+using ::testing::UnorderedElementsAre;
+
class HeadersTest : public ::testing::Test {
public:
HeadersTest() {
CDB.ExtraClangFlags = {SearchDirArg.c_str()};
FS.Files[MainFile] = "";
+// Make sure directory sub/ exists.
+FS.Files[testPath("sub/EMPTY")] = "";
+ }
+
+private:
+ std::unique_ptr setupClang() {
+auto Cmd = CDB.getCompileCommand(MainFile);
+assert(static_cast(Cmd));
+auto VFS = FS.getFileSystem();
+VFS->setCurrentWorkingDirectory(Cmd->Directory);
+
+std::vector Argv;
+for (const auto &S : Cmd->CommandLine)
+ Argv.push_back(S.c_str());
+auto CI = clang::createInvocationFromCommandLine(
+Argv,
+CompilerInstance::createDiagnostics(new DiagnosticOptions(),
+&IgnoreDiags, false),
+VFS);
+EXPECT_TRUE(static_cast(CI));
+CI->getFrontendOpts().DisableFree = false;
+
+// The diagnostic options must be set before creating a CompilerInstance.
+CI->getDiagnosticOpts().IgnoreWarnings = true;
+auto Clang = prepareCompilerInstance(
+std::move(CI), /*Preamble=*/nullptr,
+llvm::MemoryBuffer::getMemBuffer(FS.Files[MainFile], MainFile),
+std::make_shared(), VFS, IgnoreDiags);
+
+EXPECT_FALSE(Clang->getFrontendOpts().Inputs.empty());
+return Clang;
}
protected:
+ std::vector collectIncludes() {
+auto Clang = setupClang();
+PreprocessOnlyAction Action;
+EXPECT_TRUE(
+Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0]));
+std::vector Inclusions;
+Clang->getPreprocessor().addPPCallbacks(collectInclusionsInMainFileCallback(
+Clang->getSourceManager(),
+[&](Inclusion Inc) { Inclusions.push_back(std::move(Inc)); }));
+EXPECT_TRUE(Action.Execute());
+Action.EndSourceFile();
+return Inclusions;
+ }
+
// Calculates the include path, or returns "" on error.
std::string calculate(PathRef Original, PathRef Preferred = "",
+const std::vector &Inclusions = {},
bool ExpectError = false) {
+auto Clang = setupClang();
+PreprocessOnlyAction Action;
+EXPECT_TRUE(
+Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0]));
+
if (Preferred.empty())
Preferred = Original;
-auto VFS = FS.getFileSystem();
-auto Cmd = CDB.getCompileCommand(MainFile);
-assert(static_cast(Cmd));
-VFS->setCurrentWorkingDirectory(Cmd->Directory);
auto ToHeaderFile = [](llvm::StringRef Header) {
return HeaderFile{Header,
/*Verbatim=*/!llvm::sys::path::is_absolute(Header)};
};
-auto Path = calculateIncludePath(MainFile, FS.Files[MainFile],
- ToHeaderFile(Original),
- ToHeaderFile(Preferred), *Cmd, VFS);
+
+auto Path = calculateIncludePath(
+MainFile, CDB.getCompileCommand(MainFile)->Directory,
+Clang->getPreprocessor().getHeaderSearchInfo(), Inclusions,
+ToHeaderFile(Original), ToHeaderFile(Preferred));
+Action.EndSourceFile();
if (!Path) {
llvm::consumeError(Path.takeError());
EXPECT_TRUE(ExpectError);
@@ -49,52 +108,50 @@
}
return std::move(*Path);
}
+
+ Expected>
+ insert(const HeaderFile &DeclaringHeader, const HeaderFile &InsertedHeader,
+ const std::vector &ExistingInclusions = {}) {
+auto Clang = setupClang();
+PreprocessOnlyAction Action;
+EXPECT_TRUE(
+Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0]));
+
+IncludeInserter Inserter(MainFile, /*Code=*/"", format::getLLVMStyle(),
+
[PATCH] D46497: [clangd] Populate #include insertions as additional edits in completion items.
ioeric updated this revision to Diff 146839.
ioeric added a comment.
- Rebase...
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D46497
Files:
clangd/ClangdServer.cpp
clangd/CodeComplete.cpp
clangd/CodeComplete.h
clangd/Headers.cpp
clangd/Headers.h
clangd/Protocol.h
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/HeadersTests.cpp
Index: unittests/clangd/HeadersTests.cpp
===
--- unittests/clangd/HeadersTests.cpp
+++ unittests/clangd/HeadersTests.cpp
@@ -8,38 +8,97 @@
//===--===//
#include "Headers.h"
+
+#include "Compiler.h"
#include "TestFS.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Lex/PreprocessorOptions.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
namespace clang {
namespace clangd {
namespace {
+using ::testing::AllOf;
+using ::testing::UnorderedElementsAre;
+
class HeadersTest : public ::testing::Test {
public:
HeadersTest() {
CDB.ExtraClangFlags = {SearchDirArg.c_str()};
FS.Files[MainFile] = "";
+// Make sure directory sub/ exists.
+FS.Files[testPath("sub/EMPTY")] = "";
+ }
+
+private:
+ std::unique_ptr setupClang() {
+auto Cmd = CDB.getCompileCommand(MainFile);
+assert(static_cast(Cmd));
+auto VFS = FS.getFileSystem();
+VFS->setCurrentWorkingDirectory(Cmd->Directory);
+
+std::vector Argv;
+for (const auto &S : Cmd->CommandLine)
+ Argv.push_back(S.c_str());
+auto CI = clang::createInvocationFromCommandLine(
+Argv,
+CompilerInstance::createDiagnostics(new DiagnosticOptions(),
+&IgnoreDiags, false),
+VFS);
+EXPECT_TRUE(static_cast(CI));
+CI->getFrontendOpts().DisableFree = false;
+
+// The diagnostic options must be set before creating a CompilerInstance.
+CI->getDiagnosticOpts().IgnoreWarnings = true;
+auto Clang = prepareCompilerInstance(
+std::move(CI), /*Preamble=*/nullptr,
+llvm::MemoryBuffer::getMemBuffer(FS.Files[MainFile], MainFile),
+std::make_shared(), VFS, IgnoreDiags);
+
+EXPECT_FALSE(Clang->getFrontendOpts().Inputs.empty());
+return Clang;
}
protected:
+ std::vector collectIncludes() {
+auto Clang = setupClang();
+PreprocessOnlyAction Action;
+EXPECT_TRUE(
+Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0]));
+std::vector Inclusions;
+Clang->getPreprocessor().addPPCallbacks(collectInclusionsInMainFileCallback(
+Clang->getSourceManager(),
+[&](Inclusion Inc) { Inclusions.push_back(std::move(Inc)); }));
+EXPECT_TRUE(Action.Execute());
+Action.EndSourceFile();
+return Inclusions;
+ }
+
// Calculates the include path, or returns "" on error.
std::string calculate(PathRef Original, PathRef Preferred = "",
+const std::vector &Inclusions = {},
bool ExpectError = false) {
+auto Clang = setupClang();
+PreprocessOnlyAction Action;
+EXPECT_TRUE(
+Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0]));
+
if (Preferred.empty())
Preferred = Original;
-auto VFS = FS.getFileSystem();
-auto Cmd = CDB.getCompileCommand(MainFile);
-assert(static_cast(Cmd));
-VFS->setCurrentWorkingDirectory(Cmd->Directory);
auto ToHeaderFile = [](llvm::StringRef Header) {
return HeaderFile{Header,
/*Verbatim=*/!llvm::sys::path::is_absolute(Header)};
};
-auto Path = calculateIncludePath(MainFile, FS.Files[MainFile],
- ToHeaderFile(Original),
- ToHeaderFile(Preferred), *Cmd, VFS);
+
+auto Path = calculateIncludePath(
+MainFile, CDB.getCompileCommand(MainFile)->Directory,
+Clang->getPreprocessor().getHeaderSearchInfo(), Inclusions,
+ToHeaderFile(Original), ToHeaderFile(Preferred));
+Action.EndSourceFile();
if (!Path) {
llvm::consumeError(Path.takeError());
EXPECT_TRUE(ExpectError);
@@ -49,52 +108,50 @@
}
return std::move(*Path);
}
+
+ Expected>
+ insert(const HeaderFile &DeclaringHeader, const HeaderFile &InsertedHeader,
+ const std::vector &ExistingInclusions = {}) {
+auto Clang = setupClang();
+PreprocessOnlyAction Action;
+EXPECT_TRUE(
+Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0]));
+
+IncludeInserter Inserter(MainFile, /*Code=*/"", format::getLLVMStyle(),
+ CDB.getCompileCommand(MainFile)->Directory,
+ Clang->getPreprocessor().getHeaderSearchInfo());
+for (const auto &Inc : ExistingInclusions)
+ Inserter.addExisting(Inc);
+
+auto Edit = Inserter.ins
[clang-tools-extra] r332362 - [clangd] Remove LSP command-based #include insertion.
Author: ioeric
Date: Tue May 15 08:23:53 2018
New Revision: 332362
URL: http://llvm.org/viewvc/llvm-project?rev=332362&view=rev
Log:
[clangd] Remove LSP command-based #include insertion.
Summary:
clangd will populate #include insertions as addtionalEdits in completion items.
The code completion tests in ClangdServerTest will be added back in D46497.
Reviewers: ilya-biryukov, sammccall
Reviewed By: ilya-biryukov
Subscribers: klimek, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D46676
Removed:
clang-tools-extra/trunk/test/clangd/insert-include.test
Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/Protocol.cpp
clang-tools-extra/trunk/clangd/Protocol.h
clang-tools-extra/trunk/test/clangd/initialize-params-invalid.test
clang-tools-extra/trunk/test/clangd/initialize-params.test
clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=332362&r1=332361&r2=332362&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Tue May 15 08:23:53 2018
@@ -115,9 +115,7 @@ void ClangdLSPServer::onInitialize(Initi
{"workspaceSymbolProvider", true},
{"executeCommandProvider",
json::obj{
- {"commands",
- {ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND,
- ExecuteCommandParams::CLANGD_INSERT_HEADER_INCLUDE}},
+ {"commands",
{ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND}},
}},
);
}
@@ -190,42 +188,6 @@ void ClangdLSPServer::onCommand(ExecuteC
reply("Fix applied.");
ApplyEdit(*Params.workspaceEdit);
- } else if (Params.command ==
- ExecuteCommandParams::CLANGD_INSERT_HEADER_INCLUDE) {
-auto &FileURI = Params.includeInsertion->textDocument.uri;
-auto Code = DraftMgr.getDraft(FileURI.file());
-if (!Code)
- return replyError(ErrorCode::InvalidParams,
-("command " +
- ExecuteCommandParams::CLANGD_INSERT_HEADER_INCLUDE +
- " called on non-added file " + FileURI.file())
-.str());
-llvm::StringRef DeclaringHeader = Params.includeInsertion->declaringHeader;
-if (DeclaringHeader.empty())
- return replyError(
- ErrorCode::InvalidParams,
- "declaringHeader must be provided for include insertion.");
-llvm::StringRef PreferredHeader = Params.includeInsertion->preferredHeader;
-auto Replaces = Server.insertInclude(
-FileURI.file(), *Code, DeclaringHeader,
-PreferredHeader.empty() ? DeclaringHeader : PreferredHeader);
-if (!Replaces) {
- std::string ErrMsg =
- ("Failed to generate include insertion edits for adding " +
- DeclaringHeader + " (" + PreferredHeader + ") into " +
- FileURI.file())
- .str();
- log(ErrMsg + ":" + llvm::toString(Replaces.takeError()));
- replyError(ErrorCode::InternalError, ErrMsg);
- return;
-}
-auto Edits = replacementsToEdits(*Code, *Replaces);
-WorkspaceEdit WE;
-WE.changes = {{FileURI.uri(), Edits}};
-
-reply(("Inserted header " + DeclaringHeader + " (" + PreferredHeader + ")")
- .str());
-ApplyEdit(std::move(WE));
} else {
// We should not get here because ExecuteCommandParams would not have
// parsed in the first place and this handler should not be called. But if
Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=332362&r1=332361&r2=332362&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue May 15 08:23:53 2018
@@ -272,66 +272,6 @@ void ClangdServer::rename(PathRef File,
"Rename", File, Bind(Action, File.str(), NewName.str(), std::move(CB)));
}
-/// Creates a `HeaderFile` from \p Header which can be either a URI or a
literal
-/// include.
-static llvm::Expected toHeaderFile(StringRef Header,
- llvm::StringRef HintPath) {
- if (isLiteralInclude(Header))
-return HeaderFile{Header.str(), /*Verbatim=*/true};
- auto U = URI::parse(Header);
- if (!U)
-return U.takeError();
-
- auto IncludePath = URI::includeSpelling(*U);
- if (!IncludePath)
-return IncludePath.takeError();
- if (!Inc
[PATCH] D46676: [clangd] Remove LSP command-based #include insertion.
This revision was automatically updated to reflect the committed changes.
ioeric marked an inline comment as done.
Closed by commit rL332362: [clangd] Remove LSP command-based #include
insertion. (authored by ioeric, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D46676
Files:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/Protocol.cpp
clang-tools-extra/trunk/clangd/Protocol.h
clang-tools-extra/trunk/test/clangd/initialize-params-invalid.test
clang-tools-extra/trunk/test/clangd/initialize-params.test
clang-tools-extra/trunk/test/clangd/insert-include.test
clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
Index: clang-tools-extra/trunk/clangd/Protocol.h
===
--- clang-tools-extra/trunk/clangd/Protocol.h
+++ clang-tools-extra/trunk/clangd/Protocol.h
@@ -538,26 +538,6 @@
bool fromJSON(const json::Expr &, WorkspaceEdit &);
json::Expr toJSON(const WorkspaceEdit &WE);
-struct IncludeInsertion {
- /// The document in which the command was invoked.
- /// If either originalHeader or preferredHeader has been (directly) included
- /// in the current file, no new include will be inserted.
- TextDocumentIdentifier textDocument;
-
- /// The declaring header corresponding to this insertion e.g. the header that
- /// declares a symbol. This could be either a URI or a literal string quoted
- /// with <> or "" that can be #included directly.
- std::string declaringHeader;
- /// The preferred header to be inserted. This may be different from
- /// originalHeader as a header file can have a different canonical include.
- /// This could be either a URI or a literal string quoted with <> or "" that
- /// can be #included directly. If empty, declaringHeader is used to calculate
- /// the #include path.
- std::string preferredHeader;
-};
-bool fromJSON(const json::Expr &, IncludeInsertion &);
-json::Expr toJSON(const IncludeInsertion &II);
-
/// Exact commands are not specified in the protocol so we define the
/// ones supported by Clangd here. The protocol specifies the command arguments
/// to be "any[]" but to make this safer and more manageable, each command we
@@ -569,16 +549,12 @@
struct ExecuteCommandParams {
// Command to apply fix-its. Uses WorkspaceEdit as argument.
const static llvm::StringLiteral CLANGD_APPLY_FIX_COMMAND;
- // Command to insert an #include into code.
- const static llvm::StringLiteral CLANGD_INSERT_HEADER_INCLUDE;
/// The command identifier, e.g. CLANGD_APPLY_FIX_COMMAND
std::string command;
// Arguments
llvm::Optional workspaceEdit;
-
- llvm::Optional includeInsertion;
};
bool fromJSON(const json::Expr &, ExecuteCommandParams &);
@@ -750,7 +726,6 @@
/// themselves.
std::vector additionalTextEdits;
- llvm::Optional command;
// TODO(krasimir): The following optional fields defined by the language
// server protocol are unsupported:
//
Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp
===
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp
@@ -289,27 +289,6 @@
I.documentation = D->Documentation;
if (I.detail.empty())
I.detail = D->CompletionDetail;
-// FIXME: delay creating include insertion command to
-// "completionItem/resolve", when it is supported
-if (!D->IncludeHeader.empty()) {
- // LSP favors additionalTextEdits over command. But we are still using
- // command here because it would be expensive to calculate #include
- // insertion edits for all candidates, and the include insertion edit
- // is unlikely to conflict with the code completion edits.
- Command Cmd;
- // Command title is not added since this is not a user-facing command.
- Cmd.command = ExecuteCommandParams::CLANGD_INSERT_HEADER_INCLUDE;
- IncludeInsertion Insertion;
- // Fallback to canonical header if declaration location is invalid.
- Insertion.declaringHeader =
- IndexResult->CanonicalDeclaration.FileURI.empty()
- ? D->IncludeHeader
- : IndexResult->CanonicalDeclaration.FileURI;
- Insertion.preferredHeader = D->IncludeHeader;
- Insertion.textDocument.uri = URIForFile(FileName);
- Cmd.includeInsertion = std::move(Insertion);
- I.command = std::move(Cmd);
-}
}
}
I.scoreInfo = Scores;
Index: clang-tools-extra/trunk/clangd/ClangdServer.h
===
--- clang-tools-extra/trunk/clangd/ClangdServer.h
+++ clang-tools-extra
[PATCH] D46497: [clangd] Populate #include insertions as additional edits in completion items.
ioeric updated this revision to Diff 146843.
ioeric added a comment.
- Merged with origin/master
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D46497
Files:
clangd/ClangdServer.cpp
clangd/CodeComplete.cpp
clangd/CodeComplete.h
clangd/Headers.cpp
clangd/Headers.h
clangd/Protocol.h
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/HeadersTests.cpp
Index: unittests/clangd/HeadersTests.cpp
===
--- unittests/clangd/HeadersTests.cpp
+++ unittests/clangd/HeadersTests.cpp
@@ -8,38 +8,97 @@
//===--===//
#include "Headers.h"
+
+#include "Compiler.h"
#include "TestFS.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Lex/PreprocessorOptions.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
namespace clang {
namespace clangd {
namespace {
+using ::testing::AllOf;
+using ::testing::UnorderedElementsAre;
+
class HeadersTest : public ::testing::Test {
public:
HeadersTest() {
CDB.ExtraClangFlags = {SearchDirArg.c_str()};
FS.Files[MainFile] = "";
+// Make sure directory sub/ exists.
+FS.Files[testPath("sub/EMPTY")] = "";
+ }
+
+private:
+ std::unique_ptr setupClang() {
+auto Cmd = CDB.getCompileCommand(MainFile);
+assert(static_cast(Cmd));
+auto VFS = FS.getFileSystem();
+VFS->setCurrentWorkingDirectory(Cmd->Directory);
+
+std::vector Argv;
+for (const auto &S : Cmd->CommandLine)
+ Argv.push_back(S.c_str());
+auto CI = clang::createInvocationFromCommandLine(
+Argv,
+CompilerInstance::createDiagnostics(new DiagnosticOptions(),
+&IgnoreDiags, false),
+VFS);
+EXPECT_TRUE(static_cast(CI));
+CI->getFrontendOpts().DisableFree = false;
+
+// The diagnostic options must be set before creating a CompilerInstance.
+CI->getDiagnosticOpts().IgnoreWarnings = true;
+auto Clang = prepareCompilerInstance(
+std::move(CI), /*Preamble=*/nullptr,
+llvm::MemoryBuffer::getMemBuffer(FS.Files[MainFile], MainFile),
+std::make_shared(), VFS, IgnoreDiags);
+
+EXPECT_FALSE(Clang->getFrontendOpts().Inputs.empty());
+return Clang;
}
protected:
+ std::vector collectIncludes() {
+auto Clang = setupClang();
+PreprocessOnlyAction Action;
+EXPECT_TRUE(
+Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0]));
+std::vector Inclusions;
+Clang->getPreprocessor().addPPCallbacks(collectInclusionsInMainFileCallback(
+Clang->getSourceManager(),
+[&](Inclusion Inc) { Inclusions.push_back(std::move(Inc)); }));
+EXPECT_TRUE(Action.Execute());
+Action.EndSourceFile();
+return Inclusions;
+ }
+
// Calculates the include path, or returns "" on error.
std::string calculate(PathRef Original, PathRef Preferred = "",
+const std::vector &Inclusions = {},
bool ExpectError = false) {
+auto Clang = setupClang();
+PreprocessOnlyAction Action;
+EXPECT_TRUE(
+Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0]));
+
if (Preferred.empty())
Preferred = Original;
-auto VFS = FS.getFileSystem();
-auto Cmd = CDB.getCompileCommand(MainFile);
-assert(static_cast(Cmd));
-VFS->setCurrentWorkingDirectory(Cmd->Directory);
auto ToHeaderFile = [](llvm::StringRef Header) {
return HeaderFile{Header,
/*Verbatim=*/!llvm::sys::path::is_absolute(Header)};
};
-auto Path = calculateIncludePath(MainFile, FS.Files[MainFile],
- ToHeaderFile(Original),
- ToHeaderFile(Preferred), *Cmd, VFS);
+
+auto Path = calculateIncludePath(
+MainFile, CDB.getCompileCommand(MainFile)->Directory,
+Clang->getPreprocessor().getHeaderSearchInfo(), Inclusions,
+ToHeaderFile(Original), ToHeaderFile(Preferred));
+Action.EndSourceFile();
if (!Path) {
llvm::consumeError(Path.takeError());
EXPECT_TRUE(ExpectError);
@@ -49,52 +108,50 @@
}
return std::move(*Path);
}
+
+ Expected>
+ insert(const HeaderFile &DeclaringHeader, const HeaderFile &InsertedHeader,
+ const std::vector &ExistingInclusions = {}) {
+auto Clang = setupClang();
+PreprocessOnlyAction Action;
+EXPECT_TRUE(
+Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0]));
+
+IncludeInserter Inserter(MainFile, /*Code=*/"", format::getLLVMStyle(),
+ CDB.getCompileCommand(MainFile)->Directory,
+ Clang->getPreprocessor().getHeaderSearchInfo());
+for (const auto &Inc : ExistingInclusions)
+ Inserter.addExisting(Inc);
+
+auto Edi
[PATCH] D46751: [clangd] Filter out private proto symbols in SymbolCollector.
malaperle added a comment. In https://reviews.llvm.org/D46751#1099235, @sammccall wrote: > @malaperle to expand on what Eric said, adding proto hacks with false > positives and no way to turn them off is indeed not the way to go here! > There's probably going to be other places we want to filter symbols too, and > it should probably be extensible/customizable in some way. > We don't yet have enough examples to know what the structure should be > (something regex based, a code-plugin system based on `Registry`, or > something in between), thus the simplest/least invasive option for now (it's > important for our internal rollout to have *some* mitigation in place). > @ioeric can you add a comment near the proto-filtering stuff indicating we > should work out how to make this extensible? I agree with all of that. What I don't quite understand is why a flag is not ok? Just a fail-safe switch in the mean time? You can even leave it on by default so your internal service is not affected. We know for a fact that some code bases like Houdini won't work with this, at least there will be an option to make it work. In https://reviews.llvm.org/D46751#1099537, @ioeric wrote: > In https://reviews.llvm.org/D46751#1099479, @malaperle wrote: > > > In https://reviews.llvm.org/D46751#1099097, @ioeric wrote: > > > > > > I think this is good if that's true that the comment is always there. I > > > > think it would be OK for this to be enabled by default, with a general > > > > option to turn heuristics off. Not sure what to call it... > > > > -use-symbol-filtering-heuristics :) > > > > > > We have other filters in the symbol collector that we think could improve > > > user experience, and we don't provide options for those. > > > > > > What others filters do you mean? If you mean skipping "members", symbols in > > main files, etc, I a working on making them not skipped, see > > https://reviews.llvm.org/D44954. > > > I meant the filters in > https://github.com/llvm-mirror/clang-tools-extra/blob/master/clangd/index/SymbolCollector.cpp#L93 > e.g. filtering symbols in anonymous namespace, which we think should never > appear in the index. I'll be looking at adding them too. For workspaceSymbols it's useful to be able to find them and matches what we had before. But completion will not pull them from the index. > I think members are more interesting than the private proto symbols. We want > to un-filter members because there are features that would use them, so > indexing them makes sense. But private proto symbols should never be shown to > users (e.g. in code completion or workspaceSymbols). > > I also think adding an option for indexing members would actually make more > sense because they might significantly increase the index size, and it would > be good to have options to disable it if users don't use members (e.g. > including members can increase size of our internal global index service, and > we are not sure if we are ready for that). It sounds like we'll need both flags. We should discuss that because I'm planning to add even more (almost all?) symbols. I don't think it's common that users won't want members for workspaceSymbols though, but I see how this is not good for the internal indexing service. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46751 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46497: [clangd] Populate #include insertions as additional edits in completion items.
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE332363: [clangd] Populate #include insertions as
additional edits in completion items. (authored by ioeric, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D46497?vs=146843&id=146844#toc
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D46497
Files:
clangd/ClangdServer.cpp
clangd/CodeComplete.cpp
clangd/CodeComplete.h
clangd/Headers.cpp
clangd/Headers.h
clangd/Protocol.h
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/HeadersTests.cpp
Index: clangd/Headers.h
===
--- clangd/Headers.h
+++ clangd/Headers.h
@@ -12,10 +12,14 @@
#include "Path.h"
#include "Protocol.h"
+#include "SourceCode.h"
#include "clang/Basic/VirtualFileSystem.h"
+#include "clang/Format/Format.h"
+#include "clang/Lex/HeaderSearch.h"
#include "clang/Lex/PPCallbacks.h"
-#include "clang/Tooling/CompilationDatabase.h"
+#include "clang/Tooling/Core/HeaderIncludes.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSet.h"
#include "llvm/Support/Error.h"
namespace clang {
@@ -51,20 +55,51 @@
/// 'Foo/Bar.h' over a longer one like 'Baz/include/Foo/Bar.h'.
///
/// \param File is an absolute file path.
+/// \param Inclusions Existing inclusions in the main file.
/// \param DeclaringHeader is the original header corresponding to \p
/// InsertedHeader e.g. the header that declares a symbol.
/// \param InsertedHeader The preferred header to be inserted. This could be the
/// same as DeclaringHeader but must be provided.
// \return A quoted "path" or . This returns an empty string if:
/// - Either \p DeclaringHeader or \p InsertedHeader is already (directly)
-/// included in the file (including those included via different paths).
+/// in \p Inclusions (including those included via different paths).
/// - \p DeclaringHeader or \p InsertedHeader is the same as \p File.
-llvm::Expected
-calculateIncludePath(PathRef File, llvm::StringRef Code,
- const HeaderFile &DeclaringHeader,
- const HeaderFile &InsertedHeader,
- const tooling::CompileCommand &CompileCommand,
- IntrusiveRefCntPtr FS);
+llvm::Expected calculateIncludePath(
+PathRef File, StringRef BuildDir, HeaderSearch &HeaderSearchInfo,
+const std::vector &Inclusions, const HeaderFile &DeclaringHeader,
+const HeaderFile &InsertedHeader);
+
+// Calculates insertion edit for including a new header in a file.
+class IncludeInserter {
+public:
+ IncludeInserter(StringRef FileName, StringRef Code,
+ const format::FormatStyle &Style, StringRef BuildDir,
+ HeaderSearch &HeaderSearchInfo)
+ : FileName(FileName), Code(Code), BuildDir(BuildDir),
+HeaderSearchInfo(HeaderSearchInfo),
+Inserter(FileName, Code, Style.IncludeStyle) {}
+
+ void addExisting(Inclusion Inc) { Inclusions.push_back(std::move(Inc)); }
+
+ /// Returns a TextEdit that inserts a new header; if the header is not
+ /// inserted e.g. it's an existing header, this returns None. If any header is
+ /// invalid, this returns error.
+ ///
+ /// \param DeclaringHeader is the original header corresponding to \p
+ /// InsertedHeader e.g. the header that declares a symbol.
+ /// \param InsertedHeader The preferred header to be inserted. This could be
+ /// the same as DeclaringHeader but must be provided.
+ Expected> insert(const HeaderFile &DeclaringHeader,
+ const HeaderFile &InsertedHeader) const;
+
+private:
+ StringRef FileName;
+ StringRef Code;
+ StringRef BuildDir;
+ HeaderSearch &HeaderSearchInfo;
+ std::vector Inclusions;
+ tooling::HeaderIncludes Inserter; // Computers insertion replacement.
+};
} // namespace clangd
} // namespace clang
Index: clangd/CodeComplete.h
===
--- clangd/CodeComplete.h
+++ clangd/CodeComplete.h
@@ -15,6 +15,7 @@
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_CODECOMPLETE_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CODECOMPLETE_H
+#include "Headers.h"
#include "Logger.h"
#include "Path.h"
#include "Protocol.h"
@@ -69,6 +70,7 @@
CompletionList codeComplete(PathRef FileName,
const tooling::CompileCommand &Command,
PrecompiledPreamble const *Preamble,
+const std::vector &PreambleInclusions,
StringRef Contents, Position Pos,
IntrusiveRefCntPtr VFS,
std::shared_ptr PCHs,
Index: clangd/ClangdServer.cpp
===
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -161,6 +161,7 @@
// both the old and the new version in case only one of them matches.
CompletionList Res
[clang-tools-extra] r332363 - [clangd] Populate #include insertions as additional edits in completion items.
Author: ioeric
Date: Tue May 15 08:29:32 2018
New Revision: 332363
URL: http://llvm.org/viewvc/llvm-project?rev=332363&view=rev
Log:
[clangd] Populate #include insertions as additional edits in completion items.
Summary:
o Remove IncludeInsertion LSP command.
o Populate include insertion edits synchromously in completion items.
o Share the code completion compiler instance and precompiled preamble to get
existing inclusions in main file.
o Include insertion logic lives only in CodeComplete now.
o Use tooling::HeaderIncludes for inserting new includes.
o Refactored tests.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: klimek, ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D46497
Modified:
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/CodeComplete.h
clang-tools-extra/trunk/clangd/Headers.cpp
clang-tools-extra/trunk/clangd/Headers.h
clang-tools-extra/trunk/clangd/Protocol.h
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=332363&r1=332362&r2=332363&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue May 15 08:29:32 2018
@@ -161,6 +161,7 @@ void ClangdServer::codeComplete(PathRef
// both the old and the new version in case only one of them matches.
CompletionList Result = clangd::codeComplete(
File, IP->Command, PreambleData ? &PreambleData->Preamble : nullptr,
+PreambleData ? PreambleData->Inclusions : std::vector(),
IP->Contents, Pos, FS, PCHs, CodeCompleteOpts);
CB(std::move(Result));
};
Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=332363&r1=332362&r2=332363&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Tue May 15 08:29:32 2018
@@ -18,9 +18,11 @@
#include "CodeCompletionStrings.h"
#include "Compiler.h"
#include "FuzzyMatch.h"
+#include "Headers.h"
#include "Logger.h"
#include "SourceCode.h"
#include "Trace.h"
+#include "URI.h"
#include "index/Index.h"
#include "clang/Format/Format.h"
#include "clang/Frontend/CompilerInstance.h"
@@ -219,6 +221,28 @@ std::string sortText(float Score, llvm::
return S;
}
+/// Creates a `HeaderFile` from \p Header which can be either a URI or a
literal
+/// include.
+static llvm::Expected toHeaderFile(StringRef Header,
+ llvm::StringRef HintPath) {
+ if (isLiteralInclude(Header))
+return HeaderFile{Header.str(), /*Verbatim=*/true};
+ auto U = URI::parse(Header);
+ if (!U)
+return U.takeError();
+
+ auto IncludePath = URI::includeSpelling(*U);
+ if (!IncludePath)
+return IncludePath.takeError();
+ if (!IncludePath->empty())
+return HeaderFile{std::move(*IncludePath), /*Verbatim=*/true};
+
+ auto Resolved = URI::resolve(*U, HintPath);
+ if (!Resolved)
+return Resolved.takeError();
+ return HeaderFile{std::move(*Resolved), /*Verbatim=*/false};
+}
+
/// A code completion result, in clang-native form.
/// It may be promoted to a CompletionItem if it's among the top-ranked
results.
struct CompletionCandidate {
@@ -255,10 +279,10 @@ struct CompletionCandidate {
}
// Builds an LSP completion item.
- CompletionItem build(llvm::StringRef FileName,
- const CompletionItemScores &Scores,
+ CompletionItem build(StringRef FileName, const CompletionItemScores &Scores,
const CodeCompleteOptions &Opts,
- CodeCompletionString *SemaCCS) const {
+ CodeCompletionString *SemaCCS,
+ const IncludeInserter *Includes) const {
assert(bool(SemaResult) == bool(SemaCCS));
CompletionItem I;
if (SemaResult) {
@@ -289,6 +313,30 @@ struct CompletionCandidate {
I.documentation = D->Documentation;
if (I.detail.empty())
I.detail = D->CompletionDetail;
+if (Includes && !D->IncludeHeader.empty()) {
+ auto Edit = [&]() -> Expected> {
+auto ResolvedDeclaring = toHeaderFile(
+IndexResult->CanonicalDeclaration.FileURI, FileName);
+if (!ResolvedDeclaring)
+ return ResolvedDeclaring.takeError();
+auto ResolvedInserted = toHeaderFile(D->IncludeHeader, FileName);
+if (!ResolvedInserted)
+ return ResolvedInserted
[PATCH] D46834: [Sema][Cxx17] Error message for C++17 static_assert(pred) without string literal
dexonsmith added a comment. (Somehow I missed Richard's comment when I replied last night, even though it preceded mine by 5 hours...) In https://reviews.llvm.org/D46834#1098727, @rsmith wrote: > This would violate our guidelines for diagnostic messages; see > https://clang.llvm.org/docs/InternalsManual.html#the-format-string > > In particular: "Take advantage of location information. The user will be able > to see the line and location of the caret, so you don’t need to tell them > that the problem is with the 4th argument to the function: just point to it." > > > Reasons why printing assert expression in absence of string literal is > > useful are: > > > > 1. Serialized diagnostics (--serialize-diagnostics) don't contain code > > snippet and thus error message lack context. > > 2. Not all tools scraping clang diagnostics use caret snippet provided by > > -fcaret-diagnostics. > > This seems like an excellent reason to fix those tools -- per our > documentation, Clang's diagnostics are not designed to be used without the > caret and snippet. > > If you want to change our diagnostic policy from "the diagnostic text plus > the line and location of the caret should be enough to identify the problem; > do not repeat information in the diagnostic that the caret portion shows" to > "the diagnostic text alone should be sufficient", that's certainly something > we can discuss, but you'll need to make the case for why that's a good change > of policy. I wasn't aware of the policy. I can't convince myself that `static_assert` should be an exception to it. It's ironic, though. What I'm concerned about are interfaces like a Vim location list, a condensed view of diagnostics that won't show the source until you select a particular diagnostic. The status quo may lead some users to leverage a macro like this to hack the location list view: #define MY_STATIC_ASSERT(X) static_assert(X, #X) getting rid of which was the motivation for the single-argument `static_assert` in the first place. Repository: rC Clang https://reviews.llvm.org/D46834 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46835: [ASTImporter] Do not try to remove invisible Decls from DeclContext
martong added a comment.
Hi Aleksei,
Thanks for reviewing this.
I could synthesize a test which exercises only the `DeclContext::removeDecl`
function. This test causes an assertion without the fix.
Removed the rest of the testcases, which are not strictly connected to this
change.
Comment at: unittests/AST/ASTImporterTest.cpp:1827
+
+TEST_P(ASTImporterTestBase, DISABLED_ImportOfRecordWithDifferentFriends) {
+ Decl *ToR1;
a.sidorin wrote:
> For this change, we should create a separate patch.
This test is disabled ATM, but I agree it would be better to bring this in when
we fix the import of friends.
Repository:
rC Clang
https://reviews.llvm.org/D46835
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46835: [ASTImporter] Do not try to remove invisible Decls from DeclContext
martong updated this revision to Diff 146845.
martong marked 4 inline comments as done.
martong added a comment.
- Add test for removeDecl, remove unrelated tests
Repository:
rC Clang
https://reviews.llvm.org/D46835
Files:
lib/AST/DeclBase.cpp
unittests/AST/ASTImporterTest.cpp
unittests/AST/MatchVerifier.h
Index: unittests/AST/MatchVerifier.h
===
--- unittests/AST/MatchVerifier.h
+++ unittests/AST/MatchVerifier.h
@@ -28,11 +28,12 @@
namespace clang {
namespace ast_matchers {
-enum Language {
+enum Language {
Lang_C,
Lang_C89,
Lang_CXX,
Lang_CXX11,
+Lang_CXX14,
Lang_OpenCL,
Lang_OBJCXX
};
@@ -113,6 +114,10 @@
Args.push_back("-std=c++11");
FileName = "input.cc";
break;
+ case Lang_CXX14:
+Args.push_back("-std=c++14");
+FileName = "input.cc";
+break;
case Lang_OpenCL:
FileName = "input.cl";
break;
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -52,6 +52,9 @@
case Lang_CXX11:
BasicArgs = {"-std=c++11", "-frtti"};
break;
+ case Lang_CXX14:
+BasicArgs = {"-std=c++14", "-frtti"};
+break;
case Lang_OpenCL:
case Lang_OBJCXX:
llvm_unreachable("Not implemented yet!");
@@ -1764,5 +1767,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,8 @@
(D->NextInContextAndBits.getPointer() || D == LastDecl));
}
+static bool shouldBeHidden(NamedDecl *D);
+
void DeclContext::removeDecl(Decl *D) {
assert(D->getLexicalDeclContext() == this &&
"decl being removed from non-lexical context");
@@ -1372,14 +1374,18 @@
}
}
}
-
+
// 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;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46835: [ASTImporter] Do not try to remove invisible Decls from DeclContext
martong updated this revision to Diff 146847.
martong added a comment.
- Remove unrelated CXX14 changes
Repository:
rC Clang
https://reviews.llvm.org/D46835
Files:
lib/AST/DeclBase.cpp
unittests/AST/ASTImporterTest.cpp
unittests/AST/MatchVerifier.h
Index: unittests/AST/MatchVerifier.h
===
--- unittests/AST/MatchVerifier.h
+++ unittests/AST/MatchVerifier.h
@@ -28,7 +28,7 @@
namespace clang {
namespace ast_matchers {
-enum Language {
+enum Language {
Lang_C,
Lang_C89,
Lang_CXX,
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1764,5 +1764,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,8 @@
(D->NextInContextAndBits.getPointer() || D == LastDecl));
}
+static bool shouldBeHidden(NamedDecl *D);
+
void DeclContext::removeDecl(Decl *D) {
assert(D->getLexicalDeclContext() == this &&
"decl being removed from non-lexical context");
@@ -1372,14 +1374,18 @@
}
}
}
-
+
// 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;
Index: unittests/AST/MatchVerifier.h
===
--- unittests/AST/MatchVerifier.h
+++ unittests/AST/MatchVerifier.h
@@ -28,7 +28,7 @@
namespace clang {
namespace ast_matchers {
-enum Language {
+enum Language {
Lang_C,
Lang_C89,
Lang_CXX,
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1764,5 +1764,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,8 @@
(D->NextInContextAndBits.getPointer() || D == LastDecl));
}
+static bool shouldBeHidden(NamedDecl *D);
+
void DeclContext::removeDecl(Decl *D) {
assert(D->getLexicalDeclContext() == this &&
"decl being removed from non-lexical context");
@@ -1372,14 +1374,18 @@
}
}
}
-
+
// 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 s
[clang-tools-extra] r332366 - [clangd] Log error message instead write to errs(). NFC
Author: ioeric
Date: Tue May 15 09:22:43 2018
New Revision: 332366
URL: http://llvm.org/viewvc/llvm-project?rev=332366&view=rev
Log:
[clangd] Log error message instead write to errs(). NFC
Modified:
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
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=332366&r1=332365&r2=332366&view=diff
==
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Tue May 15
09:22:43 2018
@@ -51,8 +51,7 @@ llvm::Optional toURI(const
if (std::error_code EC =
SM.getFileManager().getVirtualFileSystem()->makeAbsolute(
AbsolutePath))
-llvm::errs() << "Warning: could not make absolute file: '" << EC.message()
- << '\n';
+log("Warning: could not make absolute file: " + EC.message());
if (llvm::sys::path::is_absolute(AbsolutePath)) {
// Handle the symbolic link path case where the current working directory
// (getCurrentWorkingDirectory) is a symlink./ We always want to the real
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46845: [libcxx][c++17] P0083R5: Splicing Maps and Sets
mclow.lists added a comment. >> One way we could deal with this is by adding an attribute to the compiler to >> indicate "the const is a lie", that we can apply to std::pair::first, with >> the semantics being that a top-level const is ignored when determining the >> "real" type of the member (and so mutating the member after a const_cast has >> defined behavior). This would apply to all std::pairs, not just the >> node_handle case, but in practice we don't optimize on the basis of a member >> being declared const *anyway*, so this isn't such a big deal right now. > > I'm a fan of this idea, this would also have the bonus of fixing some > pre-existing type-punning between pair and pair (see > __hash_value_type and __value_type in and ). This was //supposed// to be taken care of by [class.mem]/21 (common initial sequence), but that foundered on "standard-layout". https://reviews.llvm.org/D46845 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r332370 - update two comments as suggested on https://reviews.llvm.org/D46843
Author: nico
Date: Tue May 15 09:37:00 2018
New Revision: 332370
URL: http://llvm.org/viewvc/llvm-project?rev=332370&view=rev
Log:
update two comments as suggested on https://reviews.llvm.org/D46843
Modified:
cfe/trunk/tools/clang-fuzzer/CMakeLists.txt
cfe/trunk/tools/clang-fuzzer/proto-to-cxx/CMakeLists.txt
Modified: cfe/trunk/tools/clang-fuzzer/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-fuzzer/CMakeLists.txt?rev=332370&r1=332369&r2=332370&view=diff
==
--- cfe/trunk/tools/clang-fuzzer/CMakeLists.txt (original)
+++ cfe/trunk/tools/clang-fuzzer/CMakeLists.txt Tue May 15 09:37:00 2018
@@ -9,8 +9,7 @@ elseif(LLVM_USE_SANITIZE_COVERAGE)
unset(DUMMY_MAIN)
endif()
-# Hack to bypass LLVM's cmake sources check and allow multiple libraries and
-# executables from this directory.
+# Needed by LLVM's CMake checks because this file defines multiple targets.
set(LLVM_OPTIONAL_SOURCES
ClangFuzzer.cpp
DummyClangFuzzer.cpp
Modified: cfe/trunk/tools/clang-fuzzer/proto-to-cxx/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-fuzzer/proto-to-cxx/CMakeLists.txt?rev=332370&r1=332369&r2=332370&view=diff
==
--- cfe/trunk/tools/clang-fuzzer/proto-to-cxx/CMakeLists.txt (original)
+++ cfe/trunk/tools/clang-fuzzer/proto-to-cxx/CMakeLists.txt Tue May 15
09:37:00 2018
@@ -1,8 +1,7 @@
set(LLVM_LINK_COMPONENTS ${LLVM_TARGETS_TO_BUILD})
set(CMAKE_CXX_FLAGS ${CXX_FLAGS_NOFUZZ})
-# Hack to bypass LLVM's CMake source checks so we can have both a library and
-# an executable built from this directory.
+# Needed by LLVM's CMake checks because this file defines multiple targets.
set(LLVM_OPTIONAL_SOURCES proto_to_cxx.cpp proto_to_cxx_main.cpp)
add_clang_library(clangProtoToCXX proto_to_cxx.cpp
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r332371 - [clang-tools-extra] Update uses of DEBUG macro to LLVM_DEBUG.
Author: nzaghen
Date: Tue May 15 09:37:45 2018
New Revision: 332371
URL: http://llvm.org/viewvc/llvm-project?rev=332371&view=rev
Log:
[clang-tools-extra] Update uses of DEBUG macro to LLVM_DEBUG.
The DEBUG() macro is very generic so it might clash with other projects.
The renaming was done as follows:
- git grep -l 'DEBUG' | xargs sed -i 's/\bDEBUG\s\?(/LLVM_DEBUG(/g'
- git diff -U0 master | ../clang/tools/clang-format/clang-format-diff.py -i -p1
-style LLVM
Differential Revision: https://reviews.llvm.org/D44976
Modified:
clang-tools-extra/trunk/clang-move/ClangMove.cpp
clang-tools-extra/trunk/clang-move/HelperDeclRefGraph.cpp
clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.cpp
clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
clang-tools-extra/trunk/include-fixer/SymbolIndexManager.cpp
Modified: clang-tools-extra/trunk/clang-move/ClangMove.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-move/ClangMove.cpp?rev=332371&r1=332370&r2=332371&view=diff
==
--- clang-tools-extra/trunk/clang-move/ClangMove.cpp (original)
+++ clang-tools-extra/trunk/clang-move/ClangMove.cpp Tue May 15 09:37:45 2018
@@ -680,8 +680,8 @@ void ClangMoveTool::run(const ast_matche
Result.Nodes.getNodeAs("helper_decls")) {
MovedDecls.push_back(ND);
HelperDeclarations.push_back(ND);
-DEBUG(llvm::dbgs() << "Add helper : "
- << ND->getNameAsString() << " (" << ND << ")\n");
+LLVM_DEBUG(llvm::dbgs() << "Add helper : " << ND->getNameAsString() << " ("
+<< ND << ")\n");
} else if (const auto *UD =
Result.Nodes.getNodeAs("using_decl")) {
MovedDecls.push_back(UD);
@@ -741,12 +741,12 @@ void ClangMoveTool::removeDeclsInOldFile
// We remove the helper declarations which are not used in the old.cc after
// moving the given declarations.
for (const auto *D : HelperDeclarations) {
- DEBUG(llvm::dbgs() << "Check helper is used: "
- << D->getNameAsString() << " (" << D << ")\n");
+ LLVM_DEBUG(llvm::dbgs() << "Check helper is used: "
+ << D->getNameAsString() << " (" << D << ")\n");
if (!UsedDecls.count(HelperDeclRGBuilder::getOutmostClassOrFunDecl(
D->getCanonicalDecl( {
-DEBUG(llvm::dbgs() << "Helper removed in old.cc: "
- << D->getNameAsString() << " (" << D << ")\n");
+LLVM_DEBUG(llvm::dbgs() << "Helper removed in old.cc: "
+<< D->getNameAsString() << " (" << D << ")\n");
RemovedDecls.push_back(D);
}
}
@@ -826,8 +826,8 @@ void ClangMoveTool::moveDeclsToNewFiles(
D->getCanonicalDecl(
continue;
-DEBUG(llvm::dbgs() << "Helper used in new.cc: " << D->getNameAsString()
- << " " << D << "\n");
+LLVM_DEBUG(llvm::dbgs() << "Helper used in new.cc: " <<
D->getNameAsString()
+<< " " << D << "\n");
ActualNewCCDecls.push_back(D);
}
@@ -937,7 +937,7 @@ void ClangMoveTool::onEndOfTranslationUn
moveAll(SM, Context->Spec.OldCC, Context->Spec.NewCC);
return;
}
- DEBUG(RGBuilder.getGraph()->dump());
+ LLVM_DEBUG(RGBuilder.getGraph()->dump());
moveDeclsToNewFiles();
removeDeclsInOldFiles();
}
Modified: clang-tools-extra/trunk/clang-move/HelperDeclRefGraph.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-move/HelperDeclRefGraph.cpp?rev=332371&r1=332370&r2=332371&view=diff
==
--- clang-tools-extra/trunk/clang-move/HelperDeclRefGraph.cpp (original)
+++ clang-tools-extra/trunk/clang-move/HelperDeclRefGraph.cpp Tue May 15
09:37:45 2018
@@ -116,9 +116,9 @@ void HelperDeclRGBuilder::run(
if (const auto *FuncRef = Result.Nodes.getNodeAs("func_ref")) {
const auto *DC = Result.Nodes.getNodeAs("dc");
assert(DC);
-DEBUG(llvm::dbgs() << "Find helper function usage: "
- << FuncRef->getDecl()->getNameAsString() << " ("
- << FuncRef->getDecl() << ")\n");
+LLVM_DEBUG(llvm::dbgs() << "Find helper function usage: "
+<< FuncRef->getDecl()->getNameAsString() << " ("
+<< FuncRef->getDecl() << ")\n");
RG->addEdge(
getOutmostClassOrFunDecl(DC->getCanonicalDecl()),
getOutmostClassOrFunDecl(FuncRef->getDecl()->getCanonicalDecl()));
@@ -126,9 +126,9 @@ void HelperDeclRGBuilder::run(
Result.Nodes.getNodeAs("used_class")) {
const auto *DC = Result.Nodes.getNodeAs("dc");
assert(DC);
-DEBUG(llvm::dbgs() << "Find helper class usage: "
- << UsedClass->getNameAsString() << "
[PATCH] D44976: Change DEBUG() macro to LLVM_DEBUG() in clang-tools-extra
This revision was automatically updated to reflect the committed changes.
Closed by commit rL332371: [clang-tools-extra] Update uses of DEBUG macro to
LLVM_DEBUG. (authored by nzaghen, committed by ).
Herald added subscribers: llvm-commits, klimek.
Changed prior to commit:
https://reviews.llvm.org/D44976?vs=140056&id=146860#toc
Repository:
rL LLVM
https://reviews.llvm.org/D44976
Files:
clang-tools-extra/trunk/clang-move/ClangMove.cpp
clang-tools-extra/trunk/clang-move/HelperDeclRefGraph.cpp
clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.cpp
clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
clang-tools-extra/trunk/include-fixer/SymbolIndexManager.cpp
Index: clang-tools-extra/trunk/include-fixer/SymbolIndexManager.cpp
===
--- clang-tools-extra/trunk/include-fixer/SymbolIndexManager.cpp
+++ clang-tools-extra/trunk/include-fixer/SymbolIndexManager.cpp
@@ -97,8 +97,8 @@
Symbols.insert(Symbols.end(), Res.begin(), Res.end());
}
-DEBUG(llvm::dbgs() << "Searching " << Names.back() << "... got "
- << Symbols.size() << " results...\n");
+LLVM_DEBUG(llvm::dbgs() << "Searching " << Names.back() << "... got "
+<< Symbols.size() << " results...\n");
for (auto &SymAndSig : Symbols) {
const SymbolInfo &Symbol = SymAndSig.Symbol;
Index: clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
===
--- clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
+++ clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
@@ -156,7 +156,8 @@
clang::ASTContext &context = CI->getASTContext();
std::string QueryString = QualType(T->getUnqualifiedDesugaredType(), 0)
.getAsString(context.getPrintingPolicy());
- DEBUG(llvm::dbgs() << "Query missing complete type '" << QueryString << "'");
+ LLVM_DEBUG(llvm::dbgs() << "Query missing complete type '" << QueryString
+ << "'");
// Pass an empty range here since we don't add qualifier in this case.
std::vector MatchedSymbols =
query(QueryString, "", tooling::Range());
@@ -276,7 +277,8 @@
SymbolRange = CreateToolingRange(Typo.getLoc());
}
- DEBUG(llvm::dbgs() << "TypoScopeQualifiers: " << TypoScopeString << "\n");
+ LLVM_DEBUG(llvm::dbgs() << "TypoScopeQualifiers: " << TypoScopeString
+ << "\n");
std::vector MatchedSymbols =
query(QueryString, TypoScopeString, SymbolRange);
@@ -357,12 +359,12 @@
return {};
}
- DEBUG(llvm::dbgs() << "Looking up '" << Query << "' at ");
- DEBUG(CI->getSourceManager()
-.getLocForStartOfFile(CI->getSourceManager().getMainFileID())
-.getLocWithOffset(Range.getOffset())
-.print(llvm::dbgs(), CI->getSourceManager()));
- DEBUG(llvm::dbgs() << " ...");
+ LLVM_DEBUG(llvm::dbgs() << "Looking up '" << Query << "' at ");
+ LLVM_DEBUG(CI->getSourceManager()
+ .getLocForStartOfFile(CI->getSourceManager().getMainFileID())
+ .getLocWithOffset(Range.getOffset())
+ .print(llvm::dbgs(), CI->getSourceManager()));
+ LLVM_DEBUG(llvm::dbgs() << " ...");
llvm::StringRef FileName = CI->getSourceManager().getFilename(
CI->getSourceManager().getLocForStartOfFile(
CI->getSourceManager().getMainFileID()));
@@ -390,8 +392,8 @@
if (MatchedSymbols.empty())
MatchedSymbols =
SymbolIndexMgr.search(Query, /*IsNestedSearch=*/true, FileName);
- DEBUG(llvm::dbgs() << "Having found " << MatchedSymbols.size()
- << " symbols\n");
+ LLVM_DEBUG(llvm::dbgs() << "Having found " << MatchedSymbols.size()
+ << " symbols\n");
// We store a copy of MatchedSymbols in a place where it's globally reachable.
// This is used by the standalone version of the tool.
this->MatchedSymbols = MatchedSymbols;
Index: clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.cpp
===
--- clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.cpp
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.cpp
@@ -225,7 +225,8 @@
// similar.
std::vector
FileOptionsProvider::getRawOptions(StringRef FileName) {
- DEBUG(llvm::dbgs() << "Getting options for file " << FileName << "...\n");
+ LLVM_DEBUG(llvm::dbgs() << "Getting options for file " << FileName
+ << "...\n");
assert(FS && "FS must be set.");
llvm::SmallString<128> AbsoluteFilePath(FileName);
@@ -254,8 +255,8 @@
if (Result) {
// Store cached value for all intermediate directories.
while (Path != CurrentPath) {
-DEBUG(llvm::dbgs() << "Caching configuration for path " << Path
- << ".\n");
+LLVM_DEBUG(ll
[PATCH] D46891: [StaticAnalyzer] Added a getLValue method to ProgramState for bases
Szelethus created this revision.
Szelethus added reviewers: xazax.hun, NoQ.
Herald added subscribers: cfe-commits, a.sidorin, rnkovacs, szepet, whisperity.
Herald added a reviewer: george.karpenkov.
Repository:
rC Clang
https://reviews.llvm.org/D46891
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
@@ -294,6 +294,9 @@
ProgramStateRef enterStackFrame(const CallEvent &Call,
const StackFrameContext *CalleeCtx) const;
+ /// Get the lvalue for a base class object reference.
+ Loc getLValue(const CXXBaseSpecifier &BaseSpec, const SubRegion *Super)
const;
+
/// Get the lvalue for a variable reference.
Loc getLValue(const VarDecl *D, const LocationContext *LC) const;
@@ -724,6 +727,14 @@
return this;
}
+inline Loc ProgramState::getLValue(const CXXBaseSpecifier &BaseSpec,
+ const SubRegion *Super) const {
+ const auto Base = BaseSpec.getType()->getAsCXXRecordDecl();
+ return loc::MemRegionVal(
+ getStateManager().getRegionManager().getCXXBaseObjectRegion(
+Base, Super,
BaseSpec.isVirtual()));
+}
+
inline Loc ProgramState::getLValue(const VarDecl *VD,
const LocationContext *LC) const {
return getStateManager().StoreMgr->getLValueVar(VD, LC);
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
@@ -294,6 +294,9 @@
ProgramStateRef enterStackFrame(const CallEvent &Call,
const StackFrameContext *CalleeCtx) const;
+ /// Get the lvalue for a base class object reference.
+ Loc getLValue(const CXXBaseSpecifier &BaseSpec, const SubRegion *Super) const;
+
/// Get the lvalue for a variable reference.
Loc getLValue(const VarDecl *D, const LocationContext *LC) const;
@@ -724,6 +727,14 @@
return this;
}
+inline Loc ProgramState::getLValue(const CXXBaseSpecifier &BaseSpec,
+ const SubRegion *Super) const {
+ const auto Base = BaseSpec.getType()->getAsCXXRecordDecl();
+ return loc::MemRegionVal(
+ getStateManager().getRegionManager().getCXXBaseObjectRegion(
+Base, Super, BaseSpec.isVirtual()));
+}
+
inline Loc ProgramState::getLValue(const VarDecl *VD,
const LocationContext *LC) const {
return getStateManager().StoreMgr->getLValueVar(VD, LC);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46751: [clangd] Filter out private proto symbols in SymbolCollector.
sammccall added a comment.
In https://reviews.llvm.org/D46751#1099633, @malaperle wrote:
> In https://reviews.llvm.org/D46751#1099235, @sammccall wrote:
>
> > @malaperle to expand on what Eric said, adding proto hacks with false
> > positives and no way to turn them off is indeed not the way to go here!
> > There's probably going to be other places we want to filter symbols too,
> > and it should probably be extensible/customizable in some way.
> > We don't yet have enough examples to know what the structure should be
> > (something regex based, a code-plugin system based on `Registry`, or
> > something in between), thus the simplest/least invasive option for now
> > (it's important for our internal rollout to have *some* mitigation in
> > place).
> > @ioeric can you add a comment near the proto-filtering stuff indicating we
> > should work out how to make this extensible?
>
>
> I agree with all of that. What I don't quite understand is why a flag is not
> ok? Just a fail-safe switch in the mean time? You can even leave it on by
> default so your internal service is not affected.
I think a flag doesn't solve much of the problem, and adds new ones:
- users have to find the flag, and work out how to turn it on in their editor,
and (other than embedders) few will bother. And each flag hurts the usability
of all the other flags.
- if this heuristic is usable only sometimes, that's at codebase granularity,
not user granularity. Flags don't work that way. (Static index currently has
this problem...)
- these flags end up in config files, so if we later remove the flag we'll
*completely* break clangd for such users
> We know for a fact that some code bases like Houdini won't work with this, at
> least there will be an option to make it work.
Is this still the case after the last revision (with the comment check?)
Agree we should only hardwire this on if we are confident that false positives
are vanishingly small.
Comment at: clangd/index/SymbolCollector.cpp:101
+// we check whether it starts with PROTO_HEADER_COMMENT.
+bool isPrivateProtoSymbol(const NamedDecl &ND) {
+ const auto &SM = ND.getASTContext().getSourceManager();
We're going to end up calling this code on every decl/def we see.
Am I being paranoid by thinking we should check whether the file is a proto
once, rather than doing a bunch of string matching every time?
Comment at: clangd/index/SymbolCollector.cpp:112
+
+ auto Name = ND.getName();
+ if (!Name.contains('_'))
this asserts if the name is not a simple identifier (Maybe operators or
something will trigger this?).
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D46751
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46892: [X86] Lowering adds/addus/subs/subus intrinsics to native IR (Clang part)
tkrupa created this revision.
Herald added a subscriber: cfe-commits.
tkrupa added reviewers: craig.topper, RKSimon, spatel.
This is the patch that lowers x86 intrinsics to native IR
in order to enable optimizations.
Corresponding LLVM part: https://reviews.llvm.org/D46179
Previous https://reviews.llvm.org/D44786 version was reverted due to crashes
caused by LLVM part. This revision includes a more optimal pattern for addus.
Repository:
rC Clang
https://reviews.llvm.org/D46892
Files:
lib/CodeGen/CGBuiltin.cpp
test/CodeGen/avx2-builtins.c
test/CodeGen/avx512bw-builtins.c
test/CodeGen/avx512vlbw-builtins.c
test/CodeGen/sse2-builtins.c
Index: test/CodeGen/sse2-builtins.c
===
--- test/CodeGen/sse2-builtins.c
+++ test/CodeGen/sse2-builtins.c
@@ -47,25 +47,47 @@
__m128i test_mm_adds_epi8(__m128i A, __m128i B) {
// CHECK-LABEL: test_mm_adds_epi8
- // CHECK: call <16 x i8> @llvm.x86.sse2.padds.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+ // CHECK-NOT: call <16 x i8> @llvm.x86.sse2.padds.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+ // CHECK: sext <16 x i8> %{{.*}} to <16 x i16>
+ // CHECK: sext <16 x i8> %{{.*}} to <16 x i16>
+ // CHECK: add <16 x i16> %{{.*}}, %{{.*}}
+ // CHECK: icmp sle <16 x i16> %{{.*}},
+ // CHECK: select <16 x i1> %{{.*}}, <16 x i16> %{{.*}}, <16 x i16>
+ // CHECK: icmp slt <16 x i16> %{{.*}},
+ // CHECK: select <16 x i1> %{{.*}}, <16 x i16> , <16 x i16> %{{.*}}
+ // CHECK: trunc <16 x i16> %{{.*}} to <16 x i8>
return _mm_adds_epi8(A, B);
}
__m128i test_mm_adds_epi16(__m128i A, __m128i B) {
// CHECK-LABEL: test_mm_adds_epi16
- // CHECK: call <8 x i16> @llvm.x86.sse2.padds.w(<8 x i16> %{{.*}}, <8 x i16> %{{.*}})
+ // CHECK-NOT: call <8 x i16> @llvm.x86.sse2.padds.w(<8 x i16> %{{.*}}, <8 x i16> %{{.*}})
+ // CHECK: sext <8 x i16> %{{.*}} to <8 x i32>
+ // CHECK: sext <8 x i16> %{{.*}} to <8 x i32>
+ // CHECK: add <8 x i32> %{{.*}}, %{{.*}}
+ // CHECK: icmp sle <8 x i32> %{{.*}},
+ // CHECK: select <8 x i1> %{{.*}}, <8 x i32> %{{.*}}, <8 x i32>
+ // CHECK: icmp slt <8 x i32> %{{.*}},
+ // CHECK: select <8 x i1> %{{.*}}, <8 x i32> , <8 x i32> %{{.*}}
+ // CHECK: trunc <8 x i32> %{{.*}} to <8 x i16>
return _mm_adds_epi16(A, B);
}
__m128i test_mm_adds_epu8(__m128i A, __m128i B) {
// CHECK-LABEL: test_mm_adds_epu8
- // CHECK: call <16 x i8> @llvm.x86.sse2.paddus.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+ // CHECK-NOT: call <16 x i8> @llvm.x86.sse2.paddus.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+ // CHECK: add <16 x i8> %{{.*}}, %{{.*}}
+ // CHECK: icmp ugt <16 x i8> %{{.*}}, %{{.*}}
+ // CHECK: select <16 x i1> %{{.*}}, <16 x i8> , <16 x i8> {{.*}}
return _mm_adds_epu8(A, B);
}
__m128i test_mm_adds_epu16(__m128i A, __m128i B) {
// CHECK-LABEL: test_mm_adds_epu16
- // CHECK: call <8 x i16> @llvm.x86.sse2.paddus.w(<8 x i16> %{{.*}}, <8 x i16> %{{.*}})
+ // CHECK-NOT: call <8 x i16> @llvm.x86.sse2.paddus.w(<8 x i16> %{{.*}}, <8 x i16> %{{.*}})
+ // CHECK: add <8 x i16> %{{.*}}, %{{.*}}
+ // CHECK: icmp ugt <8 x i16> %{{.*}}, %{{.*}}
+ // CHECK: select <8 x i1> %{{.*}}, <8 x i16> , <8 x i16> {{.*}}
return _mm_adds_epu16(A, B);
}
@@ -1416,25 +1438,47 @@
__m128i test_mm_subs_epi8(__m128i A, __m128i B) {
// CHECK-LABEL: test_mm_subs_epi8
- // CHECK: call <16 x i8> @llvm.x86.sse2.psubs.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+ // CHECK-NOT: call <16 x i8> @llvm.x86.sse2.psubs.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+ // CHECK: sext <16 x i8> %{{.*}} to <16 x i16>
+ // CHECK: sext <16 x i8> %{{.*}} to <16 x i16>
+ // CHECK: sub <16 x i16> %{{.*}}, %{{.*}}
+ // CHECK: icmp sle <16 x i16> %{{.*}},
+ // CHECK: select <16 x i1> %{{.*}}, <16 x i16> %{{.*}}, <16 x i16>
+ // CHECK: icmp slt <16 x i16> %{{.*}},
+ // CHECK: select <16 x i1> %{{.*}}, <16 x i16> , <16 x i16> %{{.*}}
+ // CHECK: trunc <16 x i16> %{{.*}} to <16 x i8>
return _mm_subs_epi8(A, B);
}
__m128i test_mm_subs_epi16(__m128i A, __m128i B) {
// CHECK-LABEL: test_mm_subs_epi16
- // CHECK: call <8 x i16> @llvm.x86.sse2.psubs.w(<8 x i16> %{{.*}}, <8 x i16> %{{.*}})
+ // CHECK-NOT: call <8 x i16> @llvm.x86.sse2.psubs.w(<8 x i16> %{{.*}}, <8 x i16> %{{.*}})
+ // CHECK: sext <8 x i16> %{{.*}} to <8 x i32>
+ // CHECK: sext <8 x i16> %{{.*}} to <8 x i32>
+ // CHECK: sub <8 x i32> %{{.*}}, %{{.*}}
+ // CHECK: icmp sle <8 x i32> %{{.*}},
+ // CHECK: select <8 x i1> %{{.*}}, <8 x i32> %{{.*}}, <8 x i32>
+ // CHECK: icmp slt <8 x i32> %{{.*}},
+ // CHECK: select <8 x i1> %{{.*}}, <8 x i32> , <8 x i32> %{{.*}}
+ // CHECK: trunc <8 x i32> %{{.*}} to <8 x i16>
return _mm_subs_epi16(A, B);
}
__m128i test_mm_subs_epu8(__m128i A, __m128i B) {
// CHECK-LABEL: test_mm_subs_epu8
- // CHECK: call <16 x i8> @llvm.x86.sse2.psubus.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+ // CHECK-NOT: call <16 x i8> @llvm.x86.sse2.psubus.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+
[PATCH] D46751: [clangd] Filter out private proto symbols in SymbolCollector.
malaperle added a comment. In https://reviews.llvm.org/D46751#1099786, @sammccall wrote: > In https://reviews.llvm.org/D46751#1099633, @malaperle wrote: > > > In https://reviews.llvm.org/D46751#1099235, @sammccall wrote: > > > > > @malaperle to expand on what Eric said, adding proto hacks with false > > > positives and no way to turn them off is indeed not the way to go here! > > > There's probably going to be other places we want to filter symbols too, > > > and it should probably be extensible/customizable in some way. > > > We don't yet have enough examples to know what the structure should be > > > (something regex based, a code-plugin system based on `Registry`, or > > > something in between), thus the simplest/least invasive option for now > > > (it's important for our internal rollout to have *some* mitigation in > > > place). > > > @ioeric can you add a comment near the proto-filtering stuff indicating > > > we should work out how to make this extensible? > > > > > > I agree with all of that. What I don't quite understand is why a flag is > > not ok? Just a fail-safe switch in the mean time? You can even leave it on > > by default so your internal service is not affected. > > > I think a flag doesn't solve much of the problem, and adds new ones: > > - users have to find the flag, and work out how to turn it on in their > editor, and (other than embedders) few will bother. And each flag hurts the > usability of all the other flags. > - if this heuristic is usable only sometimes, that's at codebase granularity, > not user granularity. Flags don't work that way. (Static index currently has > this problem...) > - these flags end up in config files, so if we later remove the flag we'll > *completely* break clangd for such users I don't really agree with those points, but... >> We know for a fact that some code bases like Houdini won't work with this, >> at least there will be an option to make it work. > > Is this still the case after the last revision (with the comment check?) > Agree we should only hardwire this on if we are confident that false > positives are vanishingly small. ...I hadn't noticed the latest version. I think it's safe enough in the new version that we don't need to discuss this much further until it becomes a bigger problem (more libraries, etc). Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46751 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46747: [Sema] Use dotted form of macOS version for unguarded availability FixIts
jkorous planned changes to this revision. jkorous added a comment. Sorry for me being dense here - since the output format is determined by input source code there's more work to do. I tried to change format of introduced version in couple of tests and the output mirrors the change. That basically means that whenever someone uses underscore __attribute__((availability(macosx, introduced = 10_12))) instead of dot __attribute__((availability(macosx, introduced = 10.12))) Warnings and FixIts get underscore as well. I am now thinking about just switching to dot format in ParseAvailabilityAttribute() because it's much simpler to maintain consistency that way. https://reviews.llvm.org/D46747 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41347: [libc++] Lift std::errc into a separated header
mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. Sorry this took so long. Please update `test/libcxx/double_include.sh.cpp` and commit. Repository: rCXX libc++ https://reviews.llvm.org/D41347 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46894: [Attr] Don't print implicit attributes
jdenny created this revision.
jdenny added reviewers: aaron.ballman, rsmith, hfinkel.
Fixes bug reported at:
http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20180514/228390.html
https://reviews.llvm.org/D46894
Files:
lib/AST/DeclPrinter.cpp
test/Misc/ast-print-record-decl.c
Index: test/Misc/ast-print-record-decl.c
===
--- test/Misc/ast-print-record-decl.c
+++ test/Misc/ast-print-record-decl.c
@@ -54,20 +54,34 @@
// RUN:-DKW=struct -DBASES=' : B' -o - -xc++ %s \
// RUN: | FileCheck --check-prefixes=CHECK,LLVM %s
//
-// RUN: %clang_cc1 -verify -triple x86_64-linux -ast-print -DKW=struct
-DBASES=' : B' -xc++ %s \
-// RUN: > %t.cpp
+// RUN: %clang_cc1 -verify -ast-print -DKW=struct -DBASES=' : B' -xc++ %s \
+// RUN:> %t.cpp
// RUN: FileCheck --check-prefixes=CHECK,PRINT,PRINT-CXX -DKW=struct \
// RUN: -DBASES=' : B' %s --input-file %t.cpp
//
// Now check compiling and printing of the printed file.
//
-// RUN: echo "// expected""-warning@* 10 {{'T' is deprecated}}" >> %t.cpp
-// RUN: echo "// expected""-note@* 10 {{'T' has been explicitly marked
deprecated here}}" >> %t.cpp
+// RUN: echo "// expected""-warning@* 10 {{'T' is deprecated}}" > %t.diags
+// RUN: echo "// expected""-note@* 10 {{'T' has been explicitly marked
deprecated here}}" >> %t.diags
+// RUN: cat %t.diags >> %t.cpp
//
// RUN: %clang -target x86_64-linux -Xclang -verify -S -emit-llvm -o -
%t.cpp \
// RUN: | FileCheck --check-prefixes=CHECK,LLVM %s
//
-// RUN: %clang_cc1 -verify -triple x86_64-linux -ast-print %t.cpp \
+// RUN: %clang_cc1 -verify -ast-print %t.cpp \
+// RUN: | FileCheck --check-prefixes=CHECK,PRINT,PRINT-CXX -DKW=struct \
+// RUN: -DBASES=' : B' %s
+//
+// Make sure implicit attributes aren't printed. See comments in inMemberPtr
+// for details.
+//
+// RUN: %clang_cc1 -triple i686-pc-win32 -verify -ast-print -DKW=struct \
+// RUN:-DBASES=' : B' -xc++ %s > %t.cpp
+// RUN: FileCheck --check-prefixes=CHECK,PRINT,PRINT-CXX -DKW=struct \
+// RUN: -DBASES=' : B' %s --input-file %t.cpp
+//
+// RUN: cat %t.diags >> %t.cpp
+// RUN: %clang_cc1 -triple i686-pc-win32 -verify -ast-print %t.cpp \
// RUN: | FileCheck --check-prefixes=CHECK,PRINT,PRINT-CXX -DKW=struct \
// RUN: -DBASES=' : B' %s
@@ -236,6 +250,9 @@
#ifdef __cplusplus
// PRINT-CXX-LABEL: inMemberPtr
void inMemberPtr() {
+ // Under windows, the implicit attribute __single_inheritance used to print
+ // between KW and T1 here, but that wasn't faithful to the original source.
+ //
// PRINT-CXX-NEXT: [[KW]] T1 {
// PRINT-CXX-NEXT: int i;
// PRINT-CXX-NEXT: };
Index: lib/AST/DeclPrinter.cpp
===
--- lib/AST/DeclPrinter.cpp
+++ lib/AST/DeclPrinter.cpp
@@ -215,7 +215,7 @@
if (D->hasAttrs()) {
AttrVec &Attrs = D->getAttrs();
for (auto *A : Attrs) {
- if (A->isInherited())
+ if (A->isInherited() || A->isImplicit())
continue;
switch (A->getKind()) {
#define ATTR(X)
Index: test/Misc/ast-print-record-decl.c
===
--- test/Misc/ast-print-record-decl.c
+++ test/Misc/ast-print-record-decl.c
@@ -54,20 +54,34 @@
// RUN:-DKW=struct -DBASES=' : B' -o - -xc++ %s \
// RUN: | FileCheck --check-prefixes=CHECK,LLVM %s
//
-// RUN: %clang_cc1 -verify -triple x86_64-linux -ast-print -DKW=struct -DBASES=' : B' -xc++ %s \
-// RUN: > %t.cpp
+// RUN: %clang_cc1 -verify -ast-print -DKW=struct -DBASES=' : B' -xc++ %s \
+// RUN:> %t.cpp
// RUN: FileCheck --check-prefixes=CHECK,PRINT,PRINT-CXX -DKW=struct \
// RUN: -DBASES=' : B' %s --input-file %t.cpp
//
// Now check compiling and printing of the printed file.
//
-// RUN: echo "// expected""-warning@* 10 {{'T' is deprecated}}" >> %t.cpp
-// RUN: echo "// expected""-note@* 10 {{'T' has been explicitly marked deprecated here}}" >> %t.cpp
+// RUN: echo "// expected""-warning@* 10 {{'T' is deprecated}}" > %t.diags
+// RUN: echo "// expected""-note@* 10 {{'T' has been explicitly marked deprecated here}}" >> %t.diags
+// RUN: cat %t.diags >> %t.cpp
//
// RUN: %clang -target x86_64-linux -Xclang -verify -S -emit-llvm -o - %t.cpp \
// RUN: | FileCheck --check-prefixes=CHECK,LLVM %s
//
-// RUN: %clang_cc1 -verify -triple x86_64-linux -ast-print %t.cpp \
+// RUN: %clang_cc1 -verify -ast-print %t.cpp \
+// RUN: | FileCheck --check-prefixes=CHECK,PRINT,PRINT-CXX -DKW=struct \
+// RUN: -DBASES=' : B' %s
+//
+// Make sure implicit attributes aren't printed. See comments in inMemberPtr
+// for details.
+//
+// RUN: %clang_cc1 -triple i686-pc-win32 -verify -ast-print -DKW=struct \
+// RUN:-DBASES=' : B' -xc++ %s > %t.cpp
+// RUN: FileCheck --check-
[PATCH] D46895: add AR to acronyms of clang-tidy property check
Wizard created this revision. Herald added subscribers: cfe-commits, klimek. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46895 Files: clang-tidy/objc/PropertyDeclarationCheck.cpp Index: clang-tidy/objc/PropertyDeclarationCheck.cpp === --- clang-tidy/objc/PropertyDeclarationCheck.cpp +++ clang-tidy/objc/PropertyDeclarationCheck.cpp @@ -42,6 +42,7 @@ "[2-9]G", "ACL", "API", +"AR", "ARGB", "ASCII", "BGRA", Index: clang-tidy/objc/PropertyDeclarationCheck.cpp === --- clang-tidy/objc/PropertyDeclarationCheck.cpp +++ clang-tidy/objc/PropertyDeclarationCheck.cpp @@ -42,6 +42,7 @@ "[2-9]G", "ACL", "API", +"AR", "ARGB", "ASCII", "BGRA", ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r332314 - [AST] Fix printing tag decl groups in decl contexts
Hi Hans,
On Tue, May 15, 2018 at 6:23 AM, Hans Wennborg wrote:
> This broke the ast-print-record-decl.c test on Windows, see for
> example http://lab.llvm.org:8011/builders/clang-with-thin-lto-
> windows/builds/9066
>
> One way to reproduce the failure on Linux is to pass a Windows triple
> to this ast-print command:
>
> --- a/test/Misc/ast-print-record-decl.c
> +++ b/test/Misc/ast-print-record-decl.c
> @@ -54,7 +54,7 @@
> // RUN:-DKW=struct -DBASES=' : B' -o - -xc++ %s \
> // RUN: | FileCheck --check-prefixes=CHECK,LLVM %s
> //
> -// RUN: %clang_cc1 -verify -ast-print -DKW=struct -DBASES=' : B' -xc++
> %s \
> +// RUN: %clang_cc1 -verify -triple i686-pc-win32 -ast-print
> -DKW=struct -DBASES=' : B' -xc++ %s \
> // RUN: > %t.cpp
> // RUN: FileCheck --check-prefixes=CHECK,PRINT,PRINT-CXX -DKW=struct \
> // RUN: -DBASES=' : B' %s --input-file %t.cpp
>
> What's happening is that on Windows, "__single_inheritance(1)" is
> printed before T1. But if I change the test to allow that in the
> output, it still doesn't pass as Clang doesn't seem able to parse it.
>
The underlying bug is present at least as far back as 4.0.1. The bug is
revealed by an earlier patch, r332281, simply because it introduces this
test.
There are two parts to this bug. First, implicit attributes shouldn't
print as that's not faithful to the original source. I'm addressing that at
https://reviews.llvm.org/D46894
Second, when this attribute is explicit, the "(1)" shouldn't print as it's
not part of the accepted syntax. I'll address that in another patch.
Thanks.
Joel
>
> I've worked around the problem by adding a Linux triple here in
> r332335, but someone should probably look into it properly.
>
> Thanks,
> Hans
>
> On Tue, May 15, 2018 at 2:44 AM, Joel E. Denny via cfe-commits
> wrote:
> > Author: jdenny
> > Date: Mon May 14 17:44:14 2018
> > New Revision: 332314
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=332314&view=rev
> > Log:
> > [AST] Fix printing tag decl groups in decl contexts
> >
> > For example, given:
> >
> > struct T1 {
> > struct T2 *p0;
> > };
> >
> > -ast-print produced:
> >
> > struct T1 {
> > struct T2;
> > struct T2 *p0;
> > };
> >
> > Compiling that produces a warning that the first struct T2 declaration
> > does not declare anything.
> >
> > Details:
> >
> > A tag decl group is one or more decls that share a type specifier that
> > is a tag decl (that is, a struct/union/class/enum decl). Within
> > functions, the parser builds such a tag decl group as part of a
> > DeclStmt. However, in decl contexts, such as file scope or a member
> > list, the parser does not group together the members of a tag decl
> > group. Previously, detection of tag decl groups during printing was
> > implemented but only if the tag decl was unnamed. Otherwise, as in
> > the above example, the members of the group did not print together and
> > so sometimes introduced warnings.
> >
> > This patch extends detection of tag decl groups in decl contexts to
> > any tag decl that is recorded in the AST as not free-standing.
> >
> > Reviewed by: rsmith
> >
> > Differential Revision: https://reviews.llvm.org/D45465
> >
> > Modified:
> > cfe/trunk/lib/AST/DeclPrinter.cpp
> > cfe/trunk/test/Misc/ast-print-enum-decl.c
> > cfe/trunk/test/Misc/ast-print-record-decl.c
> > cfe/trunk/test/Sema/ast-print.c
> > cfe/trunk/test/SemaCXX/ast-print.cpp
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39308: [libcxx] Keep track of heap allocated regex states
mclow.lists added a comment. Herald added subscribers: bixia, christof. What we need to do here is to here is to add a new macro `_LIBCPP_ABI_REGEX_MEMORY` in `<__config>` (around line 89) and then make these changes available only when this macro is defined. By default - we get no change. If `_LIBCPP_ABI_REGEX_MEMORY` is defined, we get the new behavior. Then we can start adding other ABI changes as well. https://reviews.llvm.org/D39308 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45900: CodeGen: Fix invalid bitcast for lifetime.start/end
rjmccall added a comment. That would work. I think it would be reasonable for AutoVarEmission to store that pointer if it makes things easier. https://reviews.llvm.org/D45900 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41458: [libc++][C++17] Elementary string conversions for integral types
mclow.lists added inline comments. Comment at: include/charconv:89 +_LIBCPP_BEGIN_NAMESPACE_STD + +enum class _LIBCPP_ENUM_VIS chars_format lichray wrote: > EricWF wrote: > > We need to hide these names when `_LIBCPP_STD_VER < 17`, since we're not > > allowed to introduce new names into namespace `std` in older dialects. > But this header is backported to C++11, so I intended to not to guard it. > But this header is backported to C++11, so I intended to not to guard it. In general, we don't provide new features for old language versions. The only time we've ever done that is for `string_view`, and I'm **still** not sure I did the right thing there. Comment at: src/charconv.cpp:34 + +#define APPEND1(i) \ +do \ I'd really like to see some numbers here showing how this (somewhat awkward) code performs better than the straightforward versions. Because maintenance is forever. Note: I'm sure that this was true on older compilers - but is it true today? Comment at: test/support/charconv_test_helpers.h:24 + +template +constexpr auto If this is supposed to be a C++17 or later header (and I'm pretty sure it is), you should add a `static_assert(TEST_STD_VER > 14, "");` to this header. Repository: rCXX libc++ https://reviews.llvm.org/D41458 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46747: [Sema] Use dotted form of macOS version for unguarded availability FixIts
erik.pilkington added a comment. > I am now thinking about just switching to dot format in > ParseAvailabilityAttribute() because it's much simpler to maintain > consistency that way. Okay, but if we're going to treat underscores as dots while printing diagnostics and fixits (which I think we should), then we shouldn't track the spelling in VersionTuple: we no longer have any use for it. The only reason that VersionTuple supports this is because of r219124, which was written so that diagnostic printing would be consistent with how the attribute was spelled. Maybe there was a good reason for this, but I can't see the radar that was linked in the commit message. Can you look up that radar to see what r219124 was actually trying to fix? I don't think we should move forward with this until we have that context. Thanks! Erik https://reviews.llvm.org/D46747 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46845: [libcxx][c++17] P0083R5: Splicing Maps and Sets
rsmith added a comment. In https://reviews.llvm.org/D46845#1099732, @mclow.lists wrote: > >> One way we could deal with this is by adding an attribute to the compiler > >> to indicate "the const is a lie", that we can apply to std::pair::first, > >> with the semantics being that a top-level const is ignored when > >> determining the "real" type of the member (and so mutating the member > >> after a const_cast has defined behavior). This would apply to all > >> std::pairs, not just the node_handle case, but in practice we don't > >> optimize on the basis of a member being declared const *anyway*, so this > >> isn't such a big deal right now. > > > > I'm a fan of this idea, this would also have the bonus of fixing some > > pre-existing type-punning between pair and pair (see > > __hash_value_type and __value_type in and ). > > This was //supposed// to be taken care of by [class.mem]/21 (common initial > sequence), but that foundered on "standard-layout". Also, the common initial sequence rule only allows loads through the wrong type, not stores, and in any case does not alter the rule that modifying a const object results in UB. https://reviews.llvm.org/D46845 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r332378 - [clangd] Extract scoring/ranking logic, and shave yaks.
Author: sammccall
Date: Tue May 15 10:43:27 2018
New Revision: 332378
URL: http://llvm.org/viewvc/llvm-project?rev=332378&view=rev
Log:
[clangd] Extract scoring/ranking logic, and shave yaks.
Summary:
Code completion scoring was embedded in CodeComplete.cpp, which is bad:
- awkward to test. The mechanisms (extracting info from index/sema) can be
unit-tested well, the policy (scoring) should be quantitatively measured.
Neither was easily possible, and debugging was hard.
The intermediate signal struct makes this easier.
- hard to reuse. This is a bug in workspaceSymbols: it just presents the
results in the index order, which is not sorted in practice, it needs to rank
them!
Also, index implementations care about scoring (both query-dependent and
independent) in order to truncate result lists appropriately.
The main yak shaved here is the build() function that had 3 variants across
unit tests is unified in TestTU.h (rather than adding a 4th variant).
Reviewers: ilya-biryukov
Subscribers: klimek, mgorny, ioeric, MaskRay, jkorous, mgrang, cfe-commits
Differential Revision: https://reviews.llvm.org/D46524
Added:
clang-tools-extra/trunk/clangd/Quality.cpp
clang-tools-extra/trunk/clangd/Quality.h
clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp
clang-tools-extra/trunk/unittests/clangd/TestTU.cpp
clang-tools-extra/trunk/unittests/clangd/TestTU.h
Modified:
clang-tools-extra/trunk/clangd/CMakeLists.txt
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp
clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
clang-tools-extra/trunk/unittests/clangd/TestFS.cpp
clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=332378&r1=332377&r2=332378&view=diff
==
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Tue May 15 10:43:27 2018
@@ -28,6 +28,7 @@ add_clang_library(clangDaemon
Logger.cpp
Protocol.cpp
ProtocolHandlers.cpp
+ Quality.cpp
SourceCode.cpp
Threading.cpp
Trace.cpp
Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=332378&r1=332377&r2=332378&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Tue May 15 10:43:27 2018
@@ -20,6 +20,7 @@
#include "FuzzyMatch.h"
#include "Headers.h"
#include "Logger.h"
+#include "Quality.h"
#include "SourceCode.h"
#include "Trace.h"
#include "URI.h"
@@ -34,6 +35,9 @@
#include "llvm/Support/Format.h"
#include
+// We log detailed candidate here if you run with -debug-only=codecomplete.
+#define DEBUG_TYPE "codecomplete"
+
namespace clang {
namespace clangd {
namespace {
@@ -192,35 +196,6 @@ getOptionalParameters(const CodeCompleti
return Result;
}
-// Produces an integer that sorts in the same order as F.
-// That is: a < b <==> encodeFloat(a) < encodeFloat(b).
-uint32_t encodeFloat(float F) {
- static_assert(std::numeric_limits::is_iec559, "");
- static_assert(sizeof(float) == sizeof(uint32_t), "");
- constexpr uint32_t TopBit = ~(~uint32_t{0} >> 1);
-
- // Get the bits of the float. Endianness is the same as for integers.
- uint32_t U;
- memcpy(&U, &F, sizeof(float));
- // IEEE 754 floats compare like sign-magnitude integers.
- if (U & TopBit)// Negative float.
-return 0 - U;// Map onto the low half of integers, order reversed.
- return U + TopBit; // Positive floats map onto the high half of integers.
-}
-
-// Returns a string that sorts in the same order as (-Score, Name), for LSP.
-std::string sortText(float Score, llvm::StringRef Name) {
- // We convert -Score to an integer, and hex-encode for readability.
- // Example: [0.5, "foo"] -> "4100foo"
- std::string S;
- llvm::raw_string_ostream OS(S);
- write_hex(OS, encodeFloat(-Score), llvm::HexPrintStyle::Lower,
-/*Width=*/2 * sizeof(Score));
- OS << Name;
- OS.flush();
- return S;
-}
-
/// Creates a `HeaderFile` from \p Header which can be either a URI or a
literal
/// include.
static llvm::Expected toHeaderFile(StringRef Header,
@@ -251,33 +226,6 @@ struct CompletionCandidate {
const CodeCompletionResult *SemaResult = nullptr;
const Symbol *IndexResult = nullptr;
- // Computes the "symbol quality" score for this completion. Higher is better.
- float score() const {
-float Score = 1;
-if (IndexResult)
- Score *= quality(*IndexResult);
-if (SemaResult) {
- // For now we just use the
[PATCH] D46524: [clangd] Extract scoring/ranking logic, and shave yaks.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL332378: [clangd] Extract scoring/ranking logic, and shave
yaks. (authored by sammccall, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D46524?vs=146591&id=146881#toc
Repository:
rL LLVM
https://reviews.llvm.org/D46524
Files:
clang-tools-extra/trunk/clangd/CMakeLists.txt
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/Quality.cpp
clang-tools-extra/trunk/clangd/Quality.h
clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp
clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp
clang-tools-extra/trunk/unittests/clangd/TestFS.cpp
clang-tools-extra/trunk/unittests/clangd/TestTU.cpp
clang-tools-extra/trunk/unittests/clangd/TestTU.h
clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp
===
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp
@@ -20,6 +20,7 @@
#include "FuzzyMatch.h"
#include "Headers.h"
#include "Logger.h"
+#include "Quality.h"
#include "SourceCode.h"
#include "Trace.h"
#include "URI.h"
@@ -34,6 +35,9 @@
#include "llvm/Support/Format.h"
#include
+// We log detailed candidate here if you run with -debug-only=codecomplete.
+#define DEBUG_TYPE "codecomplete"
+
namespace clang {
namespace clangd {
namespace {
@@ -192,35 +196,6 @@
return Result;
}
-// Produces an integer that sorts in the same order as F.
-// That is: a < b <==> encodeFloat(a) < encodeFloat(b).
-uint32_t encodeFloat(float F) {
- static_assert(std::numeric_limits::is_iec559, "");
- static_assert(sizeof(float) == sizeof(uint32_t), "");
- constexpr uint32_t TopBit = ~(~uint32_t{0} >> 1);
-
- // Get the bits of the float. Endianness is the same as for integers.
- uint32_t U;
- memcpy(&U, &F, sizeof(float));
- // IEEE 754 floats compare like sign-magnitude integers.
- if (U & TopBit)// Negative float.
-return 0 - U;// Map onto the low half of integers, order reversed.
- return U + TopBit; // Positive floats map onto the high half of integers.
-}
-
-// Returns a string that sorts in the same order as (-Score, Name), for LSP.
-std::string sortText(float Score, llvm::StringRef Name) {
- // We convert -Score to an integer, and hex-encode for readability.
- // Example: [0.5, "foo"] -> "4100foo"
- std::string S;
- llvm::raw_string_ostream OS(S);
- write_hex(OS, encodeFloat(-Score), llvm::HexPrintStyle::Lower,
-/*Width=*/2 * sizeof(Score));
- OS << Name;
- OS.flush();
- return S;
-}
-
/// Creates a `HeaderFile` from \p Header which can be either a URI or a literal
/// include.
static llvm::Expected toHeaderFile(StringRef Header,
@@ -251,33 +226,6 @@
const CodeCompletionResult *SemaResult = nullptr;
const Symbol *IndexResult = nullptr;
- // Computes the "symbol quality" score for this completion. Higher is better.
- float score() const {
-float Score = 1;
-if (IndexResult)
- Score *= quality(*IndexResult);
-if (SemaResult) {
- // For now we just use the Sema priority, mapping it onto a 0-2 interval.
- // That makes 1 neutral-ish, so we don't reward/penalize non-Sema results.
- // Priority 80 is a really bad score.
- Score *= 2 - std::min(80, SemaResult->Priority) / 40;
-
- switch (static_cast(SemaResult->Availability)) {
- case CXAvailability_Available:
-// No penalty.
-break;
- case CXAvailability_Deprecated:
-Score *= 0.1f;
-break;
- case CXAvailability_NotAccessible:
- case CXAvailability_NotAvailable:
-Score = 0;
-break;
- }
-}
-return Score;
- }
-
// Builds an LSP completion item.
CompletionItem build(StringRef FileName, const CompletionItemScores &Scores,
const CodeCompleteOptions &Opts,
@@ -346,6 +294,7 @@
return I;
}
};
+using ScoredCandidate = std::pair;
// Determine the symbol ID for a Sema code completion result, if possible.
llvm::Optional getSymbolID(const CodeCompletionResult &R) {
@@ -552,50 +501,12 @@
UniqueFunction ResultsCallback;
};
-// Tracks a bounded number of candidates with the best scores.
-class TopN {
-public:
- using value_type = std::pair;
- static constexpr size_t Unbounded = std::numeric_limits::max();
-
- TopN(size_t N) : N(N) {}
-
- // Adds a candidate to the set.
- // Returns true if a candidate was dropped to get back under N.
- bool push(value_type &&V) {
-bool Dropped = false;
-if (Heap.size() >= N) {
- Dropped = true;
- if (N > 0 && greater(V, Heap.front())) {
-std::pop_heap(Heap.begin(), Heap.end(), greater);
-Heap.b
r332380 - [OPENMP, NVPTX] Do not globalize variables with reference/pointer types.
Author: abataev
Date: Tue May 15 11:01:01 2018
New Revision: 332380
URL: http://llvm.org/viewvc/llvm-project?rev=332380&view=rev
Log:
[OPENMP, NVPTX] Do not globalize variables with reference/pointer types.
In generic data-sharing mode we do not need to globalize
variables/parameters of reference/pointer types. They already are placed
in the global memory.
Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
cfe/trunk/test/OpenMP/nvptx_target_codegen.cpp
cfe/trunk/test/OpenMP/nvptx_target_parallel_codegen.cpp
cfe/trunk/test/OpenMP/nvptx_target_parallel_num_threads_codegen.cpp
cfe/trunk/test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp
cfe/trunk/test/OpenMP/nvptx_target_teams_distribute_parallel_for_simd_codegen.cpp
Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp?rev=332380&r1=332379&r2=332380&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp Tue May 15 11:01:01 2018
@@ -220,7 +220,10 @@ class CheckVarsEscapingDeclContext final
"Parameter captured by value with variably modified type");
EscapedParameters.insert(VD);
}
-}
+} else if (VD->getType()->isAnyPointerType() ||
+ VD->getType()->isReferenceType())
+ // Do not globalize variables with reference or pointer type.
+ return;
if (VD->getType()->isVariablyModifiedType())
EscapedVariableLengthDecls.insert(VD);
else
@@ -602,9 +605,12 @@ static const Stmt *getSingleCompoundChil
}
/// Check if the parallel directive has an 'if' clause with non-constant or
-/// false condition.
-static bool hasParallelIfClause(ASTContext &Ctx,
-const OMPExecutableDirective &D) {
+/// false condition. Also, check if the number of threads is strictly specified
+/// and run those directives in non-SPMD mode.
+static bool hasParallelIfNumThreadsClause(ASTContext &Ctx,
+ const OMPExecutableDirective &D) {
+ if (D.hasClausesOfKind())
+return true;
for (const auto *C : D.getClausesOfKind()) {
OpenMPDirectiveKind NameModifier = C->getNameModifier();
if (NameModifier != OMPD_parallel && NameModifier != OMPD_unknown)
@@ -629,7 +635,7 @@ static bool hasNestedSPMDDirective(ASTCo
switch (D.getDirectiveKind()) {
case OMPD_target:
if (isOpenMPParallelDirective(DKind) &&
- !hasParallelIfClause(Ctx, *NestedDir))
+ !hasParallelIfNumThreadsClause(Ctx, *NestedDir))
return true;
if (DKind == OMPD_teams || DKind == OMPD_teams_distribute) {
Body = NestedDir->getInnermostCapturedStmt()->IgnoreContainers();
@@ -639,7 +645,7 @@ static bool hasNestedSPMDDirective(ASTCo
if (const auto *NND = dyn_cast(ChildStmt)) {
DKind = NND->getDirectiveKind();
if (isOpenMPParallelDirective(DKind) &&
- !hasParallelIfClause(Ctx, *NND))
+ !hasParallelIfNumThreadsClause(Ctx, *NND))
return true;
if (DKind == OMPD_distribute) {
Body = NestedDir->getInnermostCapturedStmt()->IgnoreContainers();
@@ -651,7 +657,7 @@ static bool hasNestedSPMDDirective(ASTCo
if (const auto *NND = dyn_cast(ChildStmt))
{
DKind = NND->getDirectiveKind();
return isOpenMPParallelDirective(DKind) &&
- !hasParallelIfClause(Ctx, *NND);
+ !hasParallelIfNumThreadsClause(Ctx, *NND);
}
}
}
@@ -659,7 +665,7 @@ static bool hasNestedSPMDDirective(ASTCo
return false;
case OMPD_target_teams:
if (isOpenMPParallelDirective(DKind) &&
- !hasParallelIfClause(Ctx, *NestedDir))
+ !hasParallelIfNumThreadsClause(Ctx, *NestedDir))
return true;
if (DKind == OMPD_distribute) {
Body = NestedDir->getInnermostCapturedStmt()->IgnoreContainers();
@@ -669,13 +675,13 @@ static bool hasNestedSPMDDirective(ASTCo
if (const auto *NND = dyn_cast(ChildStmt)) {
DKind = NND->getDirectiveKind();
return isOpenMPParallelDirective(DKind) &&
- !hasParallelIfClause(Ctx, *NND);
+ !hasParallelIfNumThreadsClause(Ctx, *NND);
}
}
return false;
case OMPD_target_teams_distribute:
return isOpenMPParallelDirective(DKind) &&
- !hasParallelIfClause(Ctx, *NestedDir);
+ !hasParallelIfNumThreadsClause(Ctx, *NestedDir);
case OMPD_target_simd:
case OMPD_target_parallel:
case OMPD_target_parallel_for:
@@ -746,7 +752,7 @@ static bool supportsSPMDExecutionMode(AS
case OMPD_target_parallel_for_simd:
case OMPD_target_teams_distribute_parallel_for:
case OMPD_target_teams_distribute_
[PATCH] D46895: add AR to acronyms of clang-tidy property check
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE332382: add AR to acronyms of clang-tidy property check (authored by Wizard, committed by ). Changed prior to commit: https://reviews.llvm.org/D46895?vs=146874&id=146886#toc Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46895 Files: clang-tidy/objc/PropertyDeclarationCheck.cpp Index: clang-tidy/objc/PropertyDeclarationCheck.cpp === --- clang-tidy/objc/PropertyDeclarationCheck.cpp +++ clang-tidy/objc/PropertyDeclarationCheck.cpp @@ -42,6 +42,7 @@ "[2-9]G", "ACL", "API", +"AR", "ARGB", "ASCII", "BGRA", Index: clang-tidy/objc/PropertyDeclarationCheck.cpp === --- clang-tidy/objc/PropertyDeclarationCheck.cpp +++ clang-tidy/objc/PropertyDeclarationCheck.cpp @@ -42,6 +42,7 @@ "[2-9]G", "ACL", "API", +"AR", "ARGB", "ASCII", "BGRA", ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r332382 - add AR to acronyms of clang-tidy property check
Author: wizard Date: Tue May 15 11:13:51 2018 New Revision: 332382 URL: http://llvm.org/viewvc/llvm-project?rev=332382&view=rev Log: add AR to acronyms of clang-tidy property check Reviewers: hokein, benhamilton Reviewed By: benhamilton Subscribers: klimek, cfe-commits Differential Revision: https://reviews.llvm.org/D46895 Modified: clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp Modified: clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp?rev=332382&r1=332381&r2=332382&view=diff == --- clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp Tue May 15 11:13:51 2018 @@ -42,6 +42,7 @@ constexpr llvm::StringLiteral DefaultSpe "[2-9]G", "ACL", "API", +"AR", "ARGB", "ASCII", "BGRA", ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46894: [Attr] Don't print implicit attributes
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! https://reviews.llvm.org/D46894 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r332383 - [Hexagon] Add driver options for subtarget features
Author: kparzysz Date: Tue May 15 11:15:59 2018 New Revision: 332383 URL: http://llvm.org/viewvc/llvm-project?rev=332383&view=rev Log: [Hexagon] Add driver options for subtarget features Added: cfe/trunk/test/Driver/hexagon-memops.c cfe/trunk/test/Driver/hexagon-nvj.c cfe/trunk/test/Driver/hexagon-nvs.c Modified: cfe/trunk/include/clang/Driver/Options.td Modified: cfe/trunk/include/clang/Driver/Options.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=332383&r1=332382&r2=332383&view=diff == --- cfe/trunk/include/clang/Driver/Options.td (original) +++ cfe/trunk/include/clang/Driver/Options.td Tue May 15 11:15:59 2018 @@ -2562,23 +2562,35 @@ def mv62 : Flag<["-"], "mv62">, Group, AliasArgs<["hexagonv62"]>; def mv65 : Flag<["-"], "mv65">, Group, Alias, AliasArgs<["hexagonv65"]>; -def mhexagon_hvx : Flag<[ "-" ], "mhvx">, Group, +def mhexagon_hvx : Flag<["-"], "mhvx">, Group, HelpText<"Enable Hexagon Vector eXtensions">; -def mhexagon_hvx_EQ : Joined<[ "-" ], "mhvx=">, +def mhexagon_hvx_EQ : Joined<["-"], "mhvx=">, Group, HelpText<"Enable Hexagon Vector eXtensions">; -def mno_hexagon_hvx : Flag<[ "-" ], "mno-hvx">, +def mno_hexagon_hvx : Flag<["-"], "mno-hvx">, Group, HelpText<"Disable Hexagon Vector eXtensions">; -def mhexagon_hvx_length_EQ : Joined<[ "-" ], "mhvx-length=">, +def mhexagon_hvx_length_EQ : Joined<["-"], "mhvx-length=">, Group, HelpText<"Set Hexagon Vector Length">, Values<"64B,128B">; def ffixed_r19: Flag<["-"], "ffixed-r19">, - HelpText<"Reserve the r19 register (Hexagon only)">; + HelpText<"Reserve register r19 (Hexagon only)">; +def mmemops : Flag<["-"], "mmemops">, Group, + Flags<[CC1Option]>, HelpText<"Enable generation of memop instructions">; +def mno_memops : Flag<["-"], "mno-memops">, Group, + Flags<[CC1Option]>, HelpText<"Disable generation of memop instructions">; def mpackets : Flag<["-"], "mpackets">, Group, Flags<[CC1Option]>, HelpText<"Enable generation of instruction packets">; def mno_packets : Flag<["-"], "mno-packets">, Group, Flags<[CC1Option]>, HelpText<"Disable generation of instruction packets">; +def mnvj : Flag<["-"], "mnvj">, Group, + Flags<[CC1Option]>, HelpText<"Enable generation of new-value jumps">; +def mno_nvj : Flag<["-"], "mno-nvj">, Group, + Flags<[CC1Option]>, HelpText<"Disable generation of new-value jumps">; +def mnvs : Flag<["-"], "mnvs">, Group, + Flags<[CC1Option]>, HelpText<"Enable generation of new-value stores">; +def mno_nvs : Flag<["-"], "mno-nvs">, Group, + Flags<[CC1Option]>, HelpText<"Disable generation of new-value stores">; // X86 feature flags Added: cfe/trunk/test/Driver/hexagon-memops.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/hexagon-memops.c?rev=332383&view=auto == --- cfe/trunk/test/Driver/hexagon-memops.c (added) +++ cfe/trunk/test/Driver/hexagon-memops.c Tue May 15 11:15:59 2018 @@ -0,0 +1,10 @@ +// RUN: %clang -target hexagon -### -mmemops %s 2>&1 \ +// RUN:| FileCheck %s -check-prefix CHECK-MEMOPS + +// RUN: %clang -target hexagon -### -mno-memops %s 2>&1 \ +// RUN:| FileCheck %s -check-prefix CHECK-NO-MEMOPS + +// CHECK-MEMOPS: "-target-feature" "+memops" + +// CHECK-NO-MEMOPS: "-target-feature" "-memops" + Added: cfe/trunk/test/Driver/hexagon-nvj.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/hexagon-nvj.c?rev=332383&view=auto == --- cfe/trunk/test/Driver/hexagon-nvj.c (added) +++ cfe/trunk/test/Driver/hexagon-nvj.c Tue May 15 11:15:59 2018 @@ -0,0 +1,10 @@ +// RUN: %clang -target hexagon -### -mnvj %s 2>&1 \ +// RUN:| FileCheck %s -check-prefix CHECK-NVJ + +// RUN: %clang -target hexagon -### -mno-nvj %s 2>&1 \ +// RUN:| FileCheck %s -check-prefix CHECK-NO-NVJ + +// CHECK-NVJ: "-target-feature" "+nvj" + +// CHECK-NO-NVJ: "-target-feature" "-nvj" + Added: cfe/trunk/test/Driver/hexagon-nvs.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/hexagon-nvs.c?rev=332383&view=auto == --- cfe/trunk/test/Driver/hexagon-nvs.c (added) +++ cfe/trunk/test/Driver/hexagon-nvs.c Tue May 15 11:15:59 2018 @@ -0,0 +1,10 @@ +// RUN: %clang -target hexagon -### -mnvs %s 2>&1 \ +// RUN:| FileCheck %s -check-prefix CHECK-NVS + +// RUN: %clang -target hexagon -### -mno-nvs %s 2>&1 \ +// RUN:| FileCheck %s -check-prefix CHECK-NO-NVS + +// CHECK-NVS: "-target-feature" "+nvs" + +// CHECK-NO-NVS: "-target-feature" "-nvs" + ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r332384 - Fixed some rtti-options tests.
Author: ssrivastava
Date: Tue May 15 11:28:42 2018
New Revision: 332384
URL: http://llvm.org/viewvc/llvm-project?rev=332384&view=rev
Log:
Fixed some rtti-options tests.
Certain tests in rtti-options.cpp are not really testing anything because they
are testing for the absence of -frtti option to the cc1 process. Since the cc1
process does not take -frtti option, these tests are passing tautologically.
The RTTI mode is enabled by default in cc1, and -fno-rtti disables it.
Therefore the correct way to check for enabling of RTTI is to check for the
absence of -fno-rtti to cc1, and the correct way to check for disabling of RTTI
is to check for the presence of -fno-rtti to cc1.
This patch fixes those tests.
Differential Revision: https://reviews.llvm.org/D46836
Modified:
cfe/trunk/test/Driver/rtti-options.cpp
Modified: cfe/trunk/test/Driver/rtti-options.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/rtti-options.cpp?rev=332384&r1=332383&r2=332384&view=diff
==
--- cfe/trunk/test/Driver/rtti-options.cpp (original)
+++ cfe/trunk/test/Driver/rtti-options.cpp Tue May 15 11:28:42 2018
@@ -5,8 +5,8 @@
// Special cases: -fcxx-exceptions in C code should warn about unused arguments
// We should also not have any rtti-related arguments
-// RUN: %clang -x c -### -target x86_64-scei-ps4 -c -fcxx-exceptions %s 2>&1 |
FileCheck -check-prefix=CHECK-UNUSED -check-prefix=CHECK-RTTI
-check-prefix=CHECK-NO-RTTI %s
-// RUN: %clang -x c -### -target x86_64-unknown-unknown -c -fcxx-exceptions %s
2>&1 | FileCheck -check-prefix=CHECK-UNUSED -check-prefix=CHECK-RTTI
-check-prefix=CHECK-NO-RTTI %s
+// RUN: %clang -x c -### -target x86_64-scei-ps4 -c -fcxx-exceptions %s 2>&1 |
FileCheck -check-prefix=CHECK-UNUSED -check-prefix=CHECK-RTTI %s
+// RUN: %clang -x c -### -target x86_64-unknown-unknown -c -fcxx-exceptions %s
2>&1 | FileCheck -check-prefix=CHECK-UNUSED -check-prefix=CHECK-RTTI %s
// Make sure we keep the last -frtti/-fno-rtti argument
// RUN: %clang -### -c -fno-rtti -frtti %s 2>&1 | FileCheck
-check-prefix=CHECK-RTTI %s
@@ -56,6 +56,6 @@
// CHECK-EXC-ERROR: invalid argument '-fno-rtti' not allowed with
'-fexceptions'
// CHECK-EXC-ERROR-CXX: invalid argument '-fno-rtti' not allowed with
'-fcxx-exceptions'
// CHECK-RTTI-NOT: "-fno-rtti"
-// CHECK-NO-RTTI-NOT: "-frtti"
+// CHECK-NO-RTTI: "-fno-rtti"
// CHECK-OK-NOT: {{warning:|error:}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46836: Fix some rtti-options tests
This revision was automatically updated to reflect the committed changes.
Closed by commit rC332384: Fixed some rtti-options tests. (authored by
ssrivastava, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D46836?vs=146632&id=146893#toc
Repository:
rC Clang
https://reviews.llvm.org/D46836
Files:
test/Driver/rtti-options.cpp
Index: test/Driver/rtti-options.cpp
===
--- test/Driver/rtti-options.cpp
+++ test/Driver/rtti-options.cpp
@@ -5,8 +5,8 @@
// Special cases: -fcxx-exceptions in C code should warn about unused arguments
// We should also not have any rtti-related arguments
-// RUN: %clang -x c -### -target x86_64-scei-ps4 -c -fcxx-exceptions %s 2>&1 |
FileCheck -check-prefix=CHECK-UNUSED -check-prefix=CHECK-RTTI
-check-prefix=CHECK-NO-RTTI %s
-// RUN: %clang -x c -### -target x86_64-unknown-unknown -c -fcxx-exceptions %s
2>&1 | FileCheck -check-prefix=CHECK-UNUSED -check-prefix=CHECK-RTTI
-check-prefix=CHECK-NO-RTTI %s
+// RUN: %clang -x c -### -target x86_64-scei-ps4 -c -fcxx-exceptions %s 2>&1 |
FileCheck -check-prefix=CHECK-UNUSED -check-prefix=CHECK-RTTI %s
+// RUN: %clang -x c -### -target x86_64-unknown-unknown -c -fcxx-exceptions %s
2>&1 | FileCheck -check-prefix=CHECK-UNUSED -check-prefix=CHECK-RTTI %s
// Make sure we keep the last -frtti/-fno-rtti argument
// RUN: %clang -### -c -fno-rtti -frtti %s 2>&1 | FileCheck
-check-prefix=CHECK-RTTI %s
@@ -56,6 +56,6 @@
// CHECK-EXC-ERROR: invalid argument '-fno-rtti' not allowed with
'-fexceptions'
// CHECK-EXC-ERROR-CXX: invalid argument '-fno-rtti' not allowed with
'-fcxx-exceptions'
// CHECK-RTTI-NOT: "-fno-rtti"
-// CHECK-NO-RTTI-NOT: "-frtti"
+// CHECK-NO-RTTI: "-fno-rtti"
// CHECK-OK-NOT: {{warning:|error:}}
Index: test/Driver/rtti-options.cpp
===
--- test/Driver/rtti-options.cpp
+++ test/Driver/rtti-options.cpp
@@ -5,8 +5,8 @@
// Special cases: -fcxx-exceptions in C code should warn about unused arguments
// We should also not have any rtti-related arguments
-// RUN: %clang -x c -### -target x86_64-scei-ps4 -c -fcxx-exceptions %s 2>&1 | FileCheck -check-prefix=CHECK-UNUSED -check-prefix=CHECK-RTTI -check-prefix=CHECK-NO-RTTI %s
-// RUN: %clang -x c -### -target x86_64-unknown-unknown -c -fcxx-exceptions %s 2>&1 | FileCheck -check-prefix=CHECK-UNUSED -check-prefix=CHECK-RTTI -check-prefix=CHECK-NO-RTTI %s
+// RUN: %clang -x c -### -target x86_64-scei-ps4 -c -fcxx-exceptions %s 2>&1 | FileCheck -check-prefix=CHECK-UNUSED -check-prefix=CHECK-RTTI %s
+// RUN: %clang -x c -### -target x86_64-unknown-unknown -c -fcxx-exceptions %s 2>&1 | FileCheck -check-prefix=CHECK-UNUSED -check-prefix=CHECK-RTTI %s
// Make sure we keep the last -frtti/-fno-rtti argument
// RUN: %clang -### -c -fno-rtti -frtti %s 2>&1 | FileCheck -check-prefix=CHECK-RTTI %s
@@ -56,6 +56,6 @@
// CHECK-EXC-ERROR: invalid argument '-fno-rtti' not allowed with '-fexceptions'
// CHECK-EXC-ERROR-CXX: invalid argument '-fno-rtti' not allowed with '-fcxx-exceptions'
// CHECK-RTTI-NOT: "-fno-rtti"
-// CHECK-NO-RTTI-NOT: "-frtti"
+// CHECK-NO-RTTI: "-fno-rtti"
// CHECK-OK-NOT: {{warning:|error:}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46541: [CodeGen] Improve diagnostics related to target attributes
craig.topper added a comment. Ping @echristo https://reviews.llvm.org/D46541 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46863: [X86] Use __builtin_convertvector to implement some of the packed integer to packed flow conversion intrinsics.
efriedma added a comment. > I'm concerned about constant folding not taking into account runtime rounding > mode Changing the rounding mode is UB without FENV_ACCESS. And with FENV_ACCESS, __builtin_convertvector should lower to @llvm.experimental.constrained.sitofp etc., which won't constant-fold. (llvm.experimental.constrained.sitofp doesn't actually exist yet, but I assume it will eventually get added.) Repository: rC Clang https://reviews.llvm.org/D46863 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46489: [HIP] Let assembler output bitcode for amdgcn
yaxunl added a comment. In https://reviews.llvm.org/D46489#1088940, @rjmccall wrote: > I think the right solution here is to make a CompileJobAction with type > TY_LLVM_BC in the first place. You should get the advice of a driver expert, > though. There is already JobAction for TY_LLVM_BC. I just want to skip the backend and assemble phase when offloading HIP. I will try achieving that through HIP action builder. https://reviews.llvm.org/D46489 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46489: [HIP] Let assembler output bitcode for amdgcn
rjmccall added a comment. In https://reviews.llvm.org/D46489#1099979, @yaxunl wrote: > In https://reviews.llvm.org/D46489#1088940, @rjmccall wrote: > > > I think the right solution here is to make a CompileJobAction with type > > TY_LLVM_BC in the first place. You should get the advice of a driver > > expert, though. > > > There is already JobAction for TY_LLVM_BC. I just want to skip the backend > and assemble phase when offloading HIP. I will try achieving that through HIP > action builder. Right, that's what I mean. This is something we already support for LTO and other purposes. You can just check what happens in the driver if you pass `-c -emit-llvm`. https://reviews.llvm.org/D46489 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
