[clang] 6775fc6 - [clang-repl] Implement partial translation units and error recovery.
Author: Vassil Vassilev
Date: 2021-07-11T10:23:41Z
New Revision: 6775fc6ffa3ca1c36b20c25fa4e7f48f81213cf2
URL:
https://github.com/llvm/llvm-project/commit/6775fc6ffa3ca1c36b20c25fa4e7f48f81213cf2
DIFF:
https://github.com/llvm/llvm-project/commit/6775fc6ffa3ca1c36b20c25fa4e7f48f81213cf2.diff
LOG: [clang-repl] Implement partial translation units and error recovery.
https://reviews.llvm.org/D96033 contained a discussion regarding efficient
modeling of error recovery. @rjmccall has outlined the key ideas:
Conceptually, we can split the translation unit into a sequence of partial
translation units (PTUs). Every declaration will be associated with a unique PTU
that owns it.
The first key insight here is that the owning PTU isn't always the "active"
(most recent) PTU, and it isn't always the PTU that the declaration
"comes from". A new declaration (that isn't a redeclaration or specialization of
anything) does belong to the active PTU. A template specialization, however,
belongs to the most recent PTU of all the declarations in its signature - mostly
that means that it can be pulled into a more recent PTU by its template
arguments.
The second key insight is that processing a PTU might extend an earlier PTU.
Rolling back the later PTU shouldn't throw that extension away. For example, if
the second PTU defines a template, and the third PTU requires that template to
be instantiated at float, that template specialization is still part of the
second PTU. Similarly, if the fifth PTU uses an inline function belonging to the
fourth, that definition still belongs to the fourth. When we go to emit code in
a new PTU, we map each declaration we have to emit back to its owning PTU and
emit it in a new module for just the extensions to that PTU. We keep track of
all the modules we've emitted for a PTU so that we can unload them all if we
decide to roll it back.
Most declarations/definitions will only refer to entities from the same or
earlier PTUs. However, it is possible (primarily by defining a
previously-declared entity, but also through templates or ADL) for an entity
that belongs to one PTU to refer to something from a later PTU. We will have to
keep track of this and prevent unwinding to later PTU when we recognize it.
Fortunately, this should be very rare; and crucially, we don't have to do the
bookkeeping for this if we've only got one PTU, e.g. in normal compilation.
Otherwise, PTUs after the first just need to record enough metadata to be able
to revert any changes they've made to declarations belonging to earlier PTUs,
e.g. to redeclaration chains or template specialization lists.
It should even eventually be possible for PTUs to provide their own slab
allocators which can be thrown away as part of rolling back the PTU. We can
maintain a notion of the active allocator and allocate things like Stmt/Expr
nodes in it, temporarily changing it to the appropriate PTU whenever we go to do
something like instantiate a function template. More care will be required when
allocating declarations and types, though.
We would want the PTU to be efficiently recoverable from a Decl; I'm not sure
how best to do that. An easy option that would cover most declarations would be
to make multiple TranslationUnitDecls and parent the declarations appropriately,
but I don't think that's good enough for things like member function templates,
since an instantiation of that would still be parented by its original class.
Maybe we can work this into the DC chain somehow, like how lexical DCs are.
We add a different kind of translation unit `TU_Incremental` which is a
complete translation unit that we might nonetheless incrementally extend later.
Because it is complete (and we might want to generate code for it), we do
perform template instantiation, but because it might be extended later, we don't
warn if it declares or uses undefined internal-linkage symbols.
This patch teaches clang-repl how to recover from errors by disconnecting the
most recent PTU and update the primary PTU lookup tables. For instance:
```./clang-repl
clang-repl> int i = 12; error;
In file included from <<< inputs >>>:1:
input_line_0:1:13: error: C++ requires a type specifier for all declarations
int i = 12; error;
^
error: Parsing failed.
clang-repl> int i = 13; extern "C" int printf(const char*,...);
clang-repl> auto r1 = printf("i=%d\n", i);
i=13
clang-repl> quit
```
Differential revision: https://reviews.llvm.org/D104918
Added:
clang/include/clang/Interpreter/PartialTranslationUnit.h
Modified:
clang/include/clang/AST/ASTContext.h
clang/include/clang/AST/Decl.h
clang/include/clang/AST/Redeclarable.h
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Interpreter/Interpreter.h
clang/include/clang/Lex/Preprocessor.h
clang/include/clang/Sema/Sema.h
clang/lib/AST/ASTContext.cpp
clang/lib/AST/Decl.cpp
clang/lib/AST/DeclBase.cpp
clang/lib/Frontend/ASTUnit.cpp
clang/lib/Frontend/CompilerI
[PATCH] D104918: [clang-repl] Implement partial translation units and error recovery.
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6775fc6ffa3c: [clang-repl] Implement partial translation
units and error recovery. (authored by v.g.vassilev).
Herald added a project: clang.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104918/new/
https://reviews.llvm.org/D104918
Files:
clang/include/clang/AST/ASTContext.h
clang/include/clang/AST/Decl.h
clang/include/clang/AST/Redeclarable.h
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Interpreter/Interpreter.h
clang/include/clang/Interpreter/PartialTranslationUnit.h
clang/include/clang/Interpreter/Transaction.h
clang/include/clang/Lex/Preprocessor.h
clang/include/clang/Sema/Sema.h
clang/lib/AST/ASTContext.cpp
clang/lib/AST/Decl.cpp
clang/lib/AST/DeclBase.cpp
clang/lib/Frontend/ASTUnit.cpp
clang/lib/Frontend/CompilerInstance.cpp
clang/lib/Interpreter/IncrementalParser.cpp
clang/lib/Interpreter/IncrementalParser.h
clang/lib/Interpreter/Interpreter.cpp
clang/lib/Sema/Sema.cpp
clang/lib/Serialization/ASTReader.cpp
clang/tools/clang-import-test/clang-import-test.cpp
clang/unittests/AST/ASTVectorTest.cpp
clang/unittests/Interpreter/IncrementalProcessingTest.cpp
clang/unittests/Interpreter/InterpreterTest.cpp
clang/unittests/Lex/PPCallbacksTest.cpp
Index: clang/unittests/Lex/PPCallbacksTest.cpp
===
--- clang/unittests/Lex/PPCallbacksTest.cpp
+++ clang/unittests/Lex/PPCallbacksTest.cpp
@@ -323,7 +323,7 @@
// according to LangOptions, so we init Parser to register opencl
// pragma handlers
ASTContext Context(OpenCLLangOpts, SourceMgr, PP.getIdentifierTable(),
- PP.getSelectorTable(), PP.getBuiltinInfo());
+ PP.getSelectorTable(), PP.getBuiltinInfo(), PP.TUKind);
Context.InitBuiltinTypes(*Target);
ASTConsumer Consumer;
Index: clang/unittests/Interpreter/InterpreterTest.cpp
===
--- clang/unittests/Interpreter/InterpreterTest.cpp
+++ clang/unittests/Interpreter/InterpreterTest.cpp
@@ -37,34 +37,41 @@
return cantFail(clang::Interpreter::create(std::move(CI)));
}
+static size_t DeclsSize(TranslationUnitDecl *PTUDecl) {
+ return std::distance(PTUDecl->decls().begin(), PTUDecl->decls().end());
+}
+
TEST(InterpreterTest, Sanity) {
std::unique_ptr Interp = createInterpreter();
- Transaction &R1(cantFail(Interp->Parse("void g(); void g() {}")));
- EXPECT_EQ(2U, R1.Decls.size());
- Transaction &R2(cantFail(Interp->Parse("int i;")));
- EXPECT_EQ(1U, R2.Decls.size());
+ using PTU = PartialTranslationUnit;
+
+ PTU &R1(cantFail(Interp->Parse("void g(); void g() {}")));
+ EXPECT_EQ(2U, DeclsSize(R1.TUPart));
+
+ PTU &R2(cantFail(Interp->Parse("int i;")));
+ EXPECT_EQ(1U, DeclsSize(R2.TUPart));
}
-static std::string DeclToString(DeclGroupRef DGR) {
- return llvm::cast(DGR.getSingleDecl())->getQualifiedNameAsString();
+static std::string DeclToString(Decl *D) {
+ return llvm::cast(D)->getQualifiedNameAsString();
}
TEST(InterpreterTest, IncrementalInputTopLevelDecls) {
std::unique_ptr Interp = createInterpreter();
- auto R1OrErr = Interp->Parse("int var1 = 42; int f() { return var1; }");
+ auto R1 = Interp->Parse("int var1 = 42; int f() { return var1; }");
// gtest doesn't expand into explicit bool conversions.
- EXPECT_TRUE(!!R1OrErr);
- auto R1 = R1OrErr->Decls;
- EXPECT_EQ(2U, R1.size());
- EXPECT_EQ("var1", DeclToString(R1[0]));
- EXPECT_EQ("f", DeclToString(R1[1]));
-
- auto R2OrErr = Interp->Parse("int var2 = f();");
- EXPECT_TRUE(!!R2OrErr);
- auto R2 = R2OrErr->Decls;
- EXPECT_EQ(1U, R2.size());
- EXPECT_EQ("var2", DeclToString(R2[0]));
+ EXPECT_TRUE(!!R1);
+ auto R1DeclRange = R1->TUPart->decls();
+ EXPECT_EQ(2U, DeclsSize(R1->TUPart));
+ EXPECT_EQ("var1", DeclToString(*R1DeclRange.begin()));
+ EXPECT_EQ("f", DeclToString(*(++R1DeclRange.begin(;
+
+ auto R2 = Interp->Parse("int var2 = f();");
+ EXPECT_TRUE(!!R2);
+ auto R2DeclRange = R2->TUPart->decls();
+ EXPECT_EQ(1U, DeclsSize(R2->TUPart));
+ EXPECT_EQ("var2", DeclToString(*R2DeclRange.begin()));
}
TEST(InterpreterTest, Errors) {
@@ -83,9 +90,8 @@
HasSubstr("error: unknown type name 'intentional_error'"));
EXPECT_EQ("Parsing failed.", llvm::toString(std::move(Err)));
-#ifdef GTEST_HAS_DEATH_TEST
- EXPECT_DEATH((void)Interp->Parse("int var1 = 42;"), "");
-#endif
+ auto RecoverErr = Interp->Parse("int var1 = 42;");
+ EXPECT_TRUE(!!RecoverErr);
}
// Here we test whether the user can mix declarations and statements. The
@@ -101,21 +107,21 @@
DiagnosticsOS, new DiagnosticOptions());
auto Interp = createInterpreter(ExtraArgs, DiagPrinter.get());
- auto R1OrErr = Interp->Parse(
+ auto R1 = Interp->Parse(
"in
[PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.
Ericson2314 updated this revision to Diff 357643.
Ericson2314 added a comment.
Herald added a subscriber: whisperity.
rebased, fixed some new DESTINATION
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D99484/new/
https://reviews.llvm.org/D99484
Files:
clang-tools-extra/clang-doc/tool/CMakeLists.txt
clang-tools-extra/clang-include-fixer/find-all-symbols/tool/CMakeLists.txt
clang-tools-extra/clang-include-fixer/tool/CMakeLists.txt
clang-tools-extra/clang-tidy/CMakeLists.txt
clang-tools-extra/clang-tidy/tool/CMakeLists.txt
clang-tools-extra/modularize/CMakeLists.txt
clang/CMakeLists.txt
clang/cmake/modules/AddClang.cmake
clang/tools/c-index-test/CMakeLists.txt
clang/tools/clang-format/CMakeLists.txt
clang/tools/clang-rename/CMakeLists.txt
clang/tools/libclang/CMakeLists.txt
clang/tools/scan-build-py/CMakeLists.txt
clang/tools/scan-build/CMakeLists.txt
clang/tools/scan-view/CMakeLists.txt
clang/utils/hmaptool/CMakeLists.txt
compiler-rt/cmake/base-config-ix.cmake
flang/CMakeLists.txt
flang/cmake/modules/AddFlang.cmake
flang/tools/f18/CMakeLists.txt
flang/tools/flang-driver/CMakeLists.txt
libc/CMakeLists.txt
libcxx/CMakeLists.txt
libcxx/cmake/Modules/HandleLibCXXABI.cmake
libcxx/include/CMakeLists.txt
libcxx/src/CMakeLists.txt
libcxxabi/CMakeLists.txt
libunwind/CMakeLists.txt
libunwind/src/CMakeLists.txt
lld/CMakeLists.txt
lld/cmake/modules/AddLLD.cmake
lld/tools/lld/CMakeLists.txt
lldb/CMakeLists.txt
lldb/cmake/modules/AddLLDB.cmake
lldb/cmake/modules/LLDBConfig.cmake
mlir/CMakeLists.txt
mlir/cmake/modules/AddMLIR.cmake
openmp/CMakeLists.txt
openmp/runtime/src/CMakeLists.txt
openmp/tools/multiplex/CMakeLists.txt
polly/CMakeLists.txt
polly/cmake/CMakeLists.txt
polly/lib/External/CMakeLists.txt
pstl/CMakeLists.txt
Index: pstl/CMakeLists.txt
===
--- pstl/CMakeLists.txt
+++ pstl/CMakeLists.txt
@@ -7,6 +7,8 @@
#===--===##
cmake_minimum_required(VERSION 3.13.4)
+include(GNUInstallDirs)
+
set(PARALLELSTL_VERSION_FILE "${CMAKE_CURRENT_SOURCE_DIR}/include/pstl/internal/pstl_config.h")
file(STRINGS "${PARALLELSTL_VERSION_FILE}" PARALLELSTL_VERSION_SOURCE REGEX "#define _PSTL_VERSION .*$")
string(REGEX REPLACE "#define _PSTL_VERSION (.*)$" "\\1" PARALLELSTL_VERSION_SOURCE "${PARALLELSTL_VERSION_SOURCE}")
@@ -86,10 +88,10 @@
"${CMAKE_CURRENT_BINARY_DIR}/ParallelSTLConfigVersion.cmake"
DESTINATION lib/cmake/ParallelSTL)
install(DIRECTORY include/
-DESTINATION include
+DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}"
PATTERN "*.in" EXCLUDE)
install(FILES "${PSTL_CONFIG_SITE_PATH}"
-DESTINATION include)
+DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}")
add_custom_target(install-pstl
COMMAND "${CMAKE_COMMAND}" -P "${PROJECT_BINARY_DIR}/cmake_install.cmake" -DCOMPONENT=ParallelSTL)
Index: polly/lib/External/CMakeLists.txt
===
--- polly/lib/External/CMakeLists.txt
+++ polly/lib/External/CMakeLists.txt
@@ -290,7 +290,7 @@
install(DIRECTORY
${ISL_SOURCE_DIR}/include/
${ISL_BINARY_DIR}/include/
- DESTINATION include/polly
+ DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/polly"
FILES_MATCHING
PATTERN "*.h"
PATTERN "CMakeFiles" EXCLUDE
Index: polly/cmake/CMakeLists.txt
===
--- polly/cmake/CMakeLists.txt
+++ polly/cmake/CMakeLists.txt
@@ -83,14 +83,15 @@
set(POLLY_CONFIG_LLVM_CMAKE_DIR "${LLVM_BINARY_DIR}/${LLVM_INSTALL_PACKAGE_DIR}")
set(POLLY_CONFIG_CMAKE_DIR "${POLLY_INSTALL_PREFIX}/${POLLY_INSTALL_PACKAGE_DIR}")
set(POLLY_CONFIG_LIBRARY_DIRS "${POLLY_INSTALL_PREFIX}/lib${LLVM_LIBDIR_SUFFIX}")
+get_filename_component(base_includedir "${CMAKE_INSTALL_INCLUDEDIR}" ABSOLUTE BASE_DIR "${POLLY_INSTALL_PREFIX}")
if (POLLY_BUNDLED_ISL)
set(POLLY_CONFIG_INCLUDE_DIRS
-"${POLLY_INSTALL_PREFIX}/include"
-"${POLLY_INSTALL_PREFIX}/include/polly"
+"${base_includedir}"
+"${base_includedir}/polly"
)
else()
set(POLLY_CONFIG_INCLUDE_DIRS
-"${POLLY_INSTALL_PREFIX}/include"
+"${base_includedir}"
${ISL_INCLUDE_DIRS}
)
endif()
@@ -100,12 +101,12 @@
foreach(tgt IN LISTS POLLY_CONFIG_EXPORTED_TARGETS)
get_target_property(tgt_type ${tgt} TYPE)
if (tgt_type STREQUAL "EXECUTABLE")
-set(tgt_prefix "bin/")
+set(tgt_prefix "${CMAKE_INSTALL_FULL_BINDIR}/")
else()
-set(tgt_prefix "lib/")
+set(tgt_prefix "${CMAKE_INSTALL_FULL_LIBDIR}/")
endif()
- set(tgt_path "${CMAKE_INSTALL_PREFIX}/${tgt_prefix}$")
+ set(tgt_path "${tgt_prefix}$")
file(RELATIVE_PATH tgt_path ${POLLY_CONFIG_CMAKE_DIR} ${tgt_path})
if (NOT tgt_type STREQUAL "INTERFACE_LIBRARY")
Index: polly/
[clang] 5922f23 - Revert "[clang-repl] Implement partial translation units and error recovery."
Author: Vassil Vassilev
Date: 2021-07-11T14:40:10Z
New Revision: 5922f234c8c95f61534160a31db15dfc10da9b60
URL:
https://github.com/llvm/llvm-project/commit/5922f234c8c95f61534160a31db15dfc10da9b60
DIFF:
https://github.com/llvm/llvm-project/commit/5922f234c8c95f61534160a31db15dfc10da9b60.diff
LOG: Revert "[clang-repl] Implement partial translation units and error
recovery."
This reverts commit 6775fc6ffa3ca1c36b20c25fa4e7f48f81213cf2.
It also reverts "[lldb] Fix compilation by adjusting to the new ASTContext
signature."
This reverts commit 03a3f86071c10a1f6cbbf7375aa6fe9d94168972.
We see some failures on the lldb infrastructure, these changes might play a role
in it. Let's revert it now and see if the bots will become green.
Ref: https://reviews.llvm.org/D104918
Added:
clang/include/clang/Interpreter/Transaction.h
Modified:
clang/include/clang/AST/ASTContext.h
clang/include/clang/AST/Decl.h
clang/include/clang/AST/Redeclarable.h
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Interpreter/Interpreter.h
clang/include/clang/Lex/Preprocessor.h
clang/include/clang/Sema/Sema.h
clang/lib/AST/ASTContext.cpp
clang/lib/AST/Decl.cpp
clang/lib/AST/DeclBase.cpp
clang/lib/Frontend/ASTUnit.cpp
clang/lib/Frontend/CompilerInstance.cpp
clang/lib/Interpreter/IncrementalParser.cpp
clang/lib/Interpreter/IncrementalParser.h
clang/lib/Interpreter/Interpreter.cpp
clang/lib/Sema/Sema.cpp
clang/lib/Serialization/ASTReader.cpp
clang/tools/clang-import-test/clang-import-test.cpp
clang/unittests/AST/ASTVectorTest.cpp
clang/unittests/Interpreter/IncrementalProcessingTest.cpp
clang/unittests/Interpreter/InterpreterTest.cpp
clang/unittests/Lex/PPCallbacksTest.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
Removed:
clang/include/clang/Interpreter/PartialTranslationUnit.h
diff --git a/clang/include/clang/AST/ASTContext.h
b/clang/include/clang/AST/ASTContext.h
index 34299581d89d4..5032f3194a9b7 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -459,7 +459,6 @@ class ASTContext : public RefCountedBase {
friend class ASTWriter;
template friend class serialization::AbstractTypeReader;
friend class CXXRecordDecl;
- friend class IncrementalParser;
/// A mapping to contain the template or declaration that
/// a variable declaration describes or was instantiated from,
@@ -568,7 +567,7 @@ class ASTContext : public RefCountedBase {
ImportDecl *FirstLocalImport = nullptr;
ImportDecl *LastLocalImport = nullptr;
- TranslationUnitDecl *TUDecl = nullptr;
+ TranslationUnitDecl *TUDecl;
mutable ExternCContextDecl *ExternCContext = nullptr;
mutable BuiltinTemplateDecl *MakeIntegerSeqDecl = nullptr;
mutable BuiltinTemplateDecl *TypePackElementDecl = nullptr;
@@ -625,7 +624,6 @@ class ASTContext : public RefCountedBase {
IdentifierTable &Idents;
SelectorTable &Selectors;
Builtin::Context &BuiltinInfo;
- const TranslationUnitKind TUKind;
mutable DeclarationNameTable DeclarationNames;
IntrusiveRefCntPtr ExternalSource;
ASTMutationListener *Listener = nullptr;
@@ -1024,18 +1022,7 @@ class ASTContext : public RefCountedBase {
/// Get the initializations to perform when importing a module, if any.
ArrayRef getModuleInitializers(Module *M);
- TranslationUnitDecl *getTranslationUnitDecl() const {
-return TUDecl->getMostRecentDecl();
- }
- void addTranslationUnitDecl() {
-assert(!TUDecl || TUKind == TU_Incremental);
-TranslationUnitDecl *NewTUDecl = TranslationUnitDecl::Create(*this);
-if (TraversalScope.empty() || TraversalScope.back() == TUDecl)
- TraversalScope = {NewTUDecl};
-if (TUDecl)
- NewTUDecl->setPreviousDecl(TUDecl);
-TUDecl = NewTUDecl;
- }
+ TranslationUnitDecl *getTranslationUnitDecl() const { return TUDecl; }
ExternCContextDecl *getExternCContextDecl() const;
BuiltinTemplateDecl *getMakeIntegerSeqDecl() const;
@@ -1112,8 +1099,7 @@ class ASTContext : public RefCountedBase {
llvm::DenseSet CUDADeviceVarODRUsedByHost;
ASTContext(LangOptions &LOpts, SourceManager &SM, IdentifierTable &idents,
- SelectorTable &sels, Builtin::Context &builtins,
- TranslationUnitKind TUKind);
+ SelectorTable &sels, Builtin::Context &builtins);
ASTContext(const ASTContext &) = delete;
ASTContext &operator=(const ASTContext &) = delete;
~ASTContext();
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 510bf89789851..d22594ae8442a 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -79,23 +79,7 @@ class UnresolvedSetImpl;
class VarTemplateDecl;
/// The top declaration context.
-class TranslationUnitDecl : public Decl,
-public DeclContext,
-
[PATCH] D98113: [Driver] Also search FilePaths for compiler-rt before falling back
jrtc27 added a comment. Ping? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98113/new/ https://reviews.llvm.org/D98113 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D98113: [Driver] Also search FilePaths for compiler-rt before falling back
luismarques accepted this revision. luismarques added a comment. This revision is now accepted and ready to land. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98113/new/ https://reviews.llvm.org/D98113 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D95588: [RISCV] Implement the MC layer support of P extension
luismarques added a comment.
This patch is nearly there! Just address the remaining review comments and it
LGTM.
BTW, please mark all addressed inline comments as done. I think a few were
missed, and it's helpful for a large patch like this.
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:953-956
+static MCRegister convertGPRToGPRPair(MCRegister Reg) {
+ assert(isGPRPair(Reg) && "Invalid register");
+ return (Reg - RISCV::X0) / 2 + RISCV::X0_ZERO;
+}
Nitpicking: perhaps this could use better terminology. You're asserting that
`Reg` is a GPRPair, and then you convert `Reg` to a GPRPair, but the return
value would fail an assert of `isGPRPair`...
Comment at: llvm/lib/Target/RISCV/RISCVRegisterInfo.td:69
+def sub_lo : SubRegIndex<32>;
+def sub_hi : SubRegIndex<32, 32>;
jrtc27 wrote:
> This assumes RV32, and is not clear it applies to register pairs
What's the best way to address this?
Comment at: llvm/test/MC/RISCV/rv32zpsfoperand-invalid.s:6-9
+# Paired register operand
+
+# CHECK-ERROR: error: invalid operand for instruction
+smal a0, a1, a2
It would be nice to have a more specific error message, indicating the need for
an even register operand. But not a deal-breaker, IMO.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95588/new/
https://reviews.llvm.org/D95588
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D105754: [PowerPC] Fix L[D|W]ARX Implementation
nemanjai requested changes to this revision.
nemanjai added a comment.
This revision now requires changes to proceed.
I believe that your failing test case is because you are attempting to emit
code for these builtins in the target independent code and the values just
happen to match up.
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:997
+static llvm::Value *emitLoadReserveIntrinsic(CodeGenFunction &CGF,
+ unsigned BuiltinID,
lkail wrote:
> Maybe rename to `emitPPCLoadReserveIntrinsic` should be more appropriate,
> since other targets also have similar load-reserve/load-linked/load-acquire
> notions and what this function does is ad hoc to PPC.
+1
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:1000
+ const CallExpr *E) {
+ Value *addr = CGF.EmitScalarExpr(E->getArg(0));
+
Nit: please follow naming conventions. Variables are to be capitalized.
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:1015
+break;
+ }
+
lkail wrote:
> Adding `default: llvm_unreachable` would be nice.
+1
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:5144
}
+ case clang::PPC::BI__builtin_ppc_ldarx:
+ case clang::PPC::BI__builtin_ppc_lwarx:
Why?
Everything else is `Builtin::BI__` but these are
`clang::PPC::BI__` for some reason.
That doesn't really make sense to me.
Also, what on earth is this doing here? This is a PPC builtin. Those should be
handled in `EmitPPCBuiltinExpr()` and not here.
Comment at:
llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-LoadReserve-StoreCond-64bit-only.ll:18
entry:
- %0 = bitcast i64* %a to i8*
- %1 = tail call i64 @llvm.ppc.ldarx(i8* %0)
- ret i64 %1
+ %0 = call i64 asm sideeffect "ldarx $0, $1", "=r,*Z,~{memory}"(i64* %a)
+ ret i64 %0
This is not the asm that the front end generates. Why would you generate one
thing in the front end and then test a different thing in the back end?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105754/new/
https://reviews.llvm.org/D105754
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D104420: thread_local support for AIX
hubert.reinterpretcast added inline comments.
Comment at: clang/test/CodeGenCXX/cxx11-thread-local-reference.cpp:13
// LINUX: @_ZTH1r ={{.*}} alias void (), void ()* @__tls_init
+// AIXX: @_ZTH1r ={{.*}} alias void (), void ()* @__tls_init
// DARWIN: @_ZTH1r = internal alias void (), void ()* @__tls_init
Fix typo. Also, if the Linux and the AIX pattern is the same, adding a
`NONDARWIN` prefix could help.
Comment at: clang/test/CodeGenCXX/cxx11-thread-local-reference.cpp:28-29
// LINUX: define weak_odr hidden i32* @_ZTW1r() [[ATTR0:#[0-9]+]] comdat {
+// AIX: define weak_odr hidden i32* @_ZTW1r() [[ATTR0:#[0-9]+]] {
// DARWIN: define cxx_fast_tlscc i32* @_ZTW1r() [[ATTR1:#[0-9]+]] {
Can use `{{.*}}` in place of `comdat ` to common up the Linux and the AIX
pattern.
Comment at: clang/test/CodeGenCXX/cxx11-thread-local.cpp:12
// RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -o - -triple
x86_64-apple-darwin12 | FileCheck --check-prefix=CHECK --check-prefix=DARWIN %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple
powerpc64-unknown-aix-xcoff | FileCheck --check-prefix=CHECK --check-prefix=AIX
%s
Minor nit: I think this should be moved to after line 5.
Comment at: clang/test/CodeGenCXX/cxx11-thread-local.cpp:18
// LINUX-DAG: @a ={{.*}} thread_local global i32 0
+// AIX-DAG: @a ={{.*}} thread_local global i32 0
// DARWIN-DAG: @a = internal thread_local global i32 0
Same comment as for the earlier file re: adding a prefix to handle the common
Linux and AIX lines.
Comment at: clang/test/CodeGenCXX/cxx11-thread-local.cpp:229
// LINUX: define linkonce_odr hidden i32* @_ZTWN1VIcE1mE() {{#[0-9]+}} comdat {
+// AIX: define linkonce_odr hidden i32* @_ZTWN1VIcE1mE() {{#[0-9]+}} {
// LINUX: br i1 icmp ne (void ()* @_ZTHN1VIcE1mE,
Same comment as earlier about `comdat`.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104420/new/
https://reviews.llvm.org/D104420
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D105083: [clangd] Ensure Ref::Container refers to an indexed symbol
nridge updated this revision to Diff 357823.
nridge added a comment.
Address review comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105083/new/
https://reviews.llvm.org/D105083
Files:
clang-tools-extra/clangd/index/SymbolCollector.cpp
clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -811,8 +811,7 @@
};
EXPECT_EQ(Container("ref1a"),
findSymbol(Symbols, "f2").ID); // function body (call)
- // FIXME: This is wrongly contained by fptr and not f2.
- EXPECT_NE(Container("ref1b"),
+ EXPECT_EQ(Container("ref1b"),
findSymbol(Symbols, "f2").ID); // function body (address-of)
EXPECT_EQ(Container("ref2"),
findSymbol(Symbols, "v1").ID); // variable initializer
Index: clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
===
--- clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
+++ clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
@@ -26,6 +26,22 @@
namespace clang {
namespace clangd {
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &Stream,
+ const CallHierarchyItem &Item) {
+ return Stream << Item.name << "@" << Item.selectionRange;
+}
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &Stream,
+ const CallHierarchyIncomingCall &Call) {
+ Stream << "{ from: " << Call.from << ", ranges: [";
+ for (const auto &R : Call.fromRanges) {
+Stream << R;
+Stream << ", ";
+ }
+ return Stream << "] }";
+}
+
namespace {
using ::testing::AllOf;
@@ -252,6 +268,40 @@
CheckCallHierarchy(*AST, CalleeC.point(), testPath("callee.cc"));
}
+TEST(CallHierarchy, CallInLocalVarDecl) {
+ // Tests that local variable declarations are not treated as callers
+ // (they're not indexed, so they can't be represented as call hierarchy
+ // items); instead, the caller should be the containing function.
+ // However, namespace-scope variable declarations should be treated as
+ // callers because those are indexed and there is no enclosing entity
+ // that would be a useful caller.
+ Annotations Source(R"cpp(
+int call^ee();
+void caller1() {
+ $call1[[callee]]();
+}
+void caller2() {
+ int localVar = $call2[[callee]]();
+}
+int caller3 = $call3[[callee]]();
+ )cpp");
+ TestTU TU = TestTU::withCode(Source.code());
+ auto AST = TU.build();
+ auto Index = TU.index();
+
+ std::vector Items =
+ prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename));
+ ASSERT_THAT(Items, ElementsAre(WithName("callee")));
+
+ auto Incoming = incomingCalls(Items[0], Index.get());
+ ASSERT_THAT(
+ Incoming,
+ ElementsAre(
+ AllOf(From(WithName("caller1")), FromRanges(Source.range("call1"))),
+ AllOf(From(WithName("caller2")), FromRanges(Source.range("call2"))),
+ AllOf(From(WithName("caller3")), FromRanges(Source.range("call3");
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -152,6 +152,31 @@
return None;
}
+// Given a ref contained in enclosing decl `Enclosing`, return
+// the decl that should be used as that ref's Ref::Container. This is
+// usually `Enclosing` itself, but in cases where `Enclosing` is not
+// indexed, we walk further up because Ref::Container should always be
+// an indexed symbol.
+// Note: we don't use DeclContext as the container as in some cases
+// it's useful to use a Decl which is not a DeclContext. For example,
+// for a ref occurring in the initializer of a namespace-scope variable,
+// it's useful to use that variable as the container, as otherwise the
+// next enclosing DeclContext would be a NamespaceDecl or TranslationUnitDecl,
+// which are both not indexed and less granular than we'd like for use cases
+// like call hierarchy.
+const Decl *getRefContainer(const Decl *Enclosing,
+const SymbolCollector::Options &Opts) {
+ while (Enclosing) {
+const NamedDecl *ND = dyn_cast_or_null(Enclosing);
+if (ND && SymbolCollector::shouldCollectSymbol(*ND, ND->getASTContext(),
+ Opts, true)) {
+ return ND;
+}
+Enclosing = dyn_cast_or_null(Enclosing->getDeclContext());
+ }
+ return nullptr;
+}
+
} // namespace
// Encapsulates decisions about how to record header paths in the index
[PATCH] D105083: [clangd] Ensure Ref::Container refers to an indexed symbol
nridge marked an inline comment as done. nridge added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:159 +// indexed, we walk further up because Ref::Container should always be +// an indexed symbol. +const Decl *getRefContainer(const Decl *Enclosing, kadircet wrote: > can you also add some info about why we are not using `DeclContext`s ? > something like: > ``` > Decls can nest under non-DeclContext nodes, in cases like initializer, where > they might be attributed to VarDecl. > Preserving that level of granularity is especially useful for initializers at > top-level, as otherwise the only context we > can attach these refs is TranslationUnitDecl, which we don't preserve in the > index. > FIXME: Maybe we should have some symbols for representing file-scopes, that > way we can show these refs as > being contained in the file-scope. > ``` > > (Last fixme bit is optional, please add that if you also think the > functionality would be more useful that way) I've added a comment to explain why we're not using`DeclContext` here. I'm not sure if your other comment is still relevant, if we agree that the current behaviour is the desirable one. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105083/new/ https://reviews.llvm.org/D105083 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D98710: [clang-tidy] Add --skip-headers, part 1 (use setTraversalScope)
chh updated this revision to Diff 357824.
chh marked 7 inline comments as done.
chh added a comment.
sync up latest clang/llvm source; more format/style changes
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D98710/new/
https://reviews.llvm.org/D98710
Files:
clang-tools-extra/clang-tidy/ClangTidy.cpp
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
clang-tools-extra/clang-tidy/ClangTidyOptions.h
clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
clang-tools-extra/test/clang-tidy/checkers/Inputs/skip-headers/my_header1.h
clang-tools-extra/test/clang-tidy/checkers/Inputs/skip-headers/my_header2.h
clang-tools-extra/test/clang-tidy/checkers/bugprone-forward-declaration-namespace.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-interfaces-global-init.cpp
clang-tools-extra/test/clang-tidy/checkers/google-namespaces.cpp
clang-tools-extra/test/clang-tidy/checkers/google-objc-function-naming.m
clang-tools-extra/test/clang-tidy/checkers/llvm-include-order.cpp
clang-tools-extra/test/clang-tidy/checkers/llvm-prefer-register-over-unsigned.cpp
clang-tools-extra/test/clang-tidy/checkers/llvmlibc-implementation-in-namespace.cpp
clang-tools-extra/test/clang-tidy/checkers/llvmlibc-restrict-system-libc-headers.cpp
clang-tools-extra/test/clang-tidy/checkers/misc-no-recursion.cpp
clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-headers-cxx03.cpp
clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-headers-cxx11.cpp
clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value-header.cpp
clang-tools-extra/test/clang-tidy/checkers/portability-restrict-system-includes-allow.cpp
clang-tools-extra/test/clang-tidy/checkers/portability-restrict-system-includes-disallow.cpp
clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-chained-conditional-assignment.cpp
clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-chained-conditional-return.cpp
clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-members.cpp
clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp
clang-tools-extra/test/clang-tidy/checkers/skip-headers.cpp
clang/include/clang/Frontend/MultiplexConsumer.h
Index: clang/include/clang/Frontend/MultiplexConsumer.h
===
--- clang/include/clang/Frontend/MultiplexConsumer.h
+++ clang/include/clang/Frontend/MultiplexConsumer.h
@@ -77,8 +77,9 @@
void InitializeSema(Sema &S) override;
void ForgetSema() override;
-private:
+protected:
std::vector> Consumers; // Owns these.
+private:
std::unique_ptr MutationListener;
std::unique_ptr DeserializationListener;
};
Index: clang-tools-extra/test/clang-tidy/checkers/skip-headers.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/skip-headers.cpp
@@ -0,0 +1,54 @@
+// Test --skip-headers, --show-all-warnings, and --header-filter.
+// TODO: when skip-headers implementation is complete, add back
+// -implicit-check-not="{{warning|error}}:"
+// and use no_hint instead of hint
+//
+// Default shows no warning in .h files, and give HINT to use --header-filter
+// RUN: clang-tidy %s -checks='*' -- \
+// RUN: 2>&1 | FileCheck %s -check-prefixes WARN,MAIN,HINT
+// later:-implicit-check-not="{{warning|error}}:"
+//
+// --skip-headers skips included files; finds only warnings in the main file.
+// RUN: clang-tidy %s -checks='*' --skip-headers -- \
+// RUN: 2>&1 | FileCheck %s -check-prefixes WARN,MAIN,HINT
+// later:no_hint -implicit-check-not="{{warning|error}}:"
+//
+// --show-all-warnings reports all warnings, even without --header-filters
+// RUN: clang-tidy %s -checks='*' --show-all-warnings -- \
+// RUN: 2>&1 | FileCheck %s -check-prefixes WARN,WARN_BOTH,MAIN,NO_HINT
+//
+// --header-filter='.*' is like --show-all-warnings
+// RUN: clang-tidy %s -checks='*' --header-filter='.*' -- \
+// RUN: 2>&1 | FileCheck %s -check-prefixes WARN,WARN_BOTH,MAIN,NO_HINT
+//
+// --header-filter='header1.h' shows only warnings in header1.h
+// RUN: clang-tidy %s -checks='*' --header-filter='header1.h' -- \
+// RUN: 2>&1 | FileCheck %s -check-prefixes WARN,WARN1,MAIN,HINT
+// later:-implicit-check-not="{{warning|error}}:"
+//
+// The main purpose of --show-all-warnings is to debug --skip-headers.
+// When used together, no warnings should be reported from header files.
+// RUN: clang-tidy %s -checks='*' --skip-headers --show-all-warnings -- \
+// RUN: 2>&1 | FileCheck %s -check-prefixes WARN,MAIN,NO_HINT
+// later: -implicit-check-not="{{warning|error}}:"
+//
+// use --skip-headers and --header-filter='header2.h'
+//
[PATCH] D98710: [clang-tidy] Add --skip-headers, part 1 (use setTraversalScope)
chh added inline comments.
Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:310
+struct DeclFilter {
+ DeclFilter(ClangTidyContext &Ctx, SourceManager &SM)
+ : Context(Ctx), Sources(SM) {
sammccall wrote:
> Ctx is ultimately unused, just take the regex instead?
Context is used in skipFileID.
Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:514
+ for (auto &Consumer : Consumers) {
+if (hasFilter() && Consumer.get() == FinderASTConsumerPtr) {
+ // Modify AST before calling the Finder's ASTConsumer.
sammccall wrote:
> This adds significant conceptual complexity and possible runtime costs to
> guard against the possibility that the static analyzer would not be
> compatible with simply setting the traversal scope.
>
> Please do not do this. It may appear that you're cheaply eliminating a class
> of bugs, but complexity is not cheap, and in the long run can also introduce
> bugs. Instead investigate whether this is a real problem and why.
>
> If it's not then `Context.setTraversalScope()` +
> `MultiplexingASTConsumer::HandleTranslationUnit` is enough here, and
> `FinderASTConsumerPtr` can go away.
For this first step implementation of skip-headers, could we limit the risk and
impact to only AST MatchFinder based checks? If it works fine, we can add more
performance improvements, handle PPCallback-based checks, and static analyzer
checks. We can turn skip-headers to default or revert any follow up step.
At this time, we only know that PPCallback-based checks can save some more time
with skip-headers, but do not know if any static analyzer check can benefit
from skip-headers easily. In other words, we are sure to deliver big saving of
runtime for clang-tidy users that do not enable static analyzer checks, but not
sure about static analyzer check users.
The overhead and complexity of set/getTraversalScope and checking filters will
be on the users of skip-header, but not on the static analyzer checkers.
If we apply the same change to AST for all consumers, the saving of code
complexity is smaller than the risk of impact to static analyzer checks, and
the saving of runtime is also much smaller than the already saved time in
MatchFinder.
Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:473
+MatchFinder::matchAST(Context);
+Context.setTraversalScope(SavedScope);
+ } else {
sammccall wrote:
> chh wrote:
> > sammccall wrote:
> > > Is restoring the traversal scope strictly needed by the static analyzer?
> > > I would expect not, but I might be wrong.
> > I think it is safer to assume low or acceptable overhead in
> > get/setTraversalScope and keep the impact only to the MatchFinder consumer,
> > rather than depending on static analyzer working now or in the future with
> > the changed AST.
> >
> > I think it is safer to assume low or acceptable overhead in
> > get/setTraversalScope
> We have reasons to believe this is not cheap. The first usage of getParent()
> after changing traversal scope incurs a full AST traversal.
>
> > rather than depending on static analyzer working now or in the future with
> > the changed AST.
>
> There are a fairly small number of reasons that setTraversalScope could break
> static analyzer at this point, and I think they're all going to cause the
> static analyzer to be slow when used with PCHes. However the static
> analyzer's implementation and comments say that it goes to effort to be fast
> with PCHes. So I do believe this is a more stable assumption, and in the long
> run we may be able to simplify its implementation.
Okay, won't assume low overhead in set/getTraversalScope.
So far their impact is limited to skip-headers, which in this step is limited
to only MatchFinder. Please see also my reply in the other comment.
Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp:99
IO.mapOptional("InheritParentConfig", Options.InheritParentConfig);
IO.mapOptional("UseColor", Options.UseColor);
}
sammccall wrote:
> Maybe an explicit comment that ShowAllHeaders, SkipHeaders are
> runtime/optimization options that are configured at the command-line only,
> and therefore not mapped here
Why SystemHeaders is not mapped?
I agree that ShowAllWarnings is not important here,
but SkipHeaders seems as important to be included here as HeaderFilterRegex and
SystemHeaders.
Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.h:84
+ /// and system files when --system-headers is used.
+ llvm::Optional SkipHeaders;
+
sammccall wrote:
> This is easy to confuse with e.g. HeaderFilterRegex and SystemHeaders, which
> control policy rather than just implementation strategy.
> I'd suggest calling this something like `PruneTraversal` which hints at the
> implementation strategy it controls but most importantly that it's *not*
[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure
nridge updated this revision to Diff 357826.
nridge marked 3 inline comments as done.
nridge added a comment.
Address review comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105457/new/
https://reviews.llvm.org/D105457
Files:
clang/unittests/AST/ASTPrint.h
clang/unittests/AST/DeclPrinterTest.cpp
clang/unittests/AST/NamedDeclPrinterTest.cpp
clang/unittests/AST/StmtPrinterTest.cpp
Index: clang/unittests/AST/StmtPrinterTest.cpp
===
--- clang/unittests/AST/StmtPrinterTest.cpp
+++ clang/unittests/AST/StmtPrinterTest.cpp
@@ -38,11 +38,29 @@
has(compoundStmt(has(stmt().bind("id");
}
+static void PrintStmt(raw_ostream &Out, const ASTContext *Context,
+ const Stmt *S, PrintingPolicyAdjuster PolicyAdjuster) {
+ assert(S != nullptr && "Expected non-null Stmt");
+ PrintingPolicy Policy = Context->getPrintingPolicy();
+ if (PolicyAdjuster)
+PolicyAdjuster(Policy);
+ S->printPretty(Out, /*Helper*/ nullptr, Policy);
+}
+
+template
+::testing::AssertionResult
+PrintedStmtMatches(StringRef Code, const std::vector &Args,
+ const Matcher &NodeMatch, StringRef ExpectedPrinted,
+ PrintingPolicyAdjuster PolicyAdjuster = nullptr) {
+ return PrintedNodeMatches(Code, Args, NodeMatch, ExpectedPrinted, "",
+ PrintStmt, PolicyAdjuster);
+}
+
template
::testing::AssertionResult
PrintedStmtCXXMatches(StdVer Standard, StringRef Code, const T &NodeMatch,
StringRef ExpectedPrinted,
- PolicyAdjusterType PolicyAdjuster = None) {
+ PrintingPolicyAdjuster PolicyAdjuster = nullptr) {
const char *StdOpt;
switch (Standard) {
case StdVer::CXX98: StdOpt = "-std=c++98"; break;
@@ -64,7 +82,7 @@
::testing::AssertionResult
PrintedStmtMSMatches(StringRef Code, const T &NodeMatch,
StringRef ExpectedPrinted,
- PolicyAdjusterType PolicyAdjuster = None) {
+ PrintingPolicyAdjuster PolicyAdjuster = nullptr) {
std::vector Args = {
"-std=c++98",
"-target", "i686-pc-win32",
@@ -79,7 +97,7 @@
::testing::AssertionResult
PrintedStmtObjCMatches(StringRef Code, const T &NodeMatch,
StringRef ExpectedPrinted,
- PolicyAdjusterType PolicyAdjuster = None) {
+ PrintingPolicyAdjuster PolicyAdjuster = nullptr) {
std::vector Args = {
"-ObjC",
"-fobjc-runtime=macosx-10.12.0",
@@ -202,10 +220,10 @@
};
)";
// No implicit 'this'.
- ASSERT_TRUE(PrintedStmtCXXMatches(StdVer::CXX11,
- CPPSource, memberExpr(anything()).bind("id"), "field",
- PolicyAdjusterType(
- [](PrintingPolicy &PP) { PP.SuppressImplicitBase = true; })));
+ ASSERT_TRUE(PrintedStmtCXXMatches(
+ StdVer::CXX11, CPPSource, memberExpr(anything()).bind("id"), "field",
+
+ [](PrintingPolicy &PP) { PP.SuppressImplicitBase = true; }));
// Print implicit 'this'.
ASSERT_TRUE(PrintedStmtCXXMatches(StdVer::CXX11,
CPPSource, memberExpr(anything()).bind("id"), "this->field"));
@@ -222,11 +240,10 @@
@end
)";
// No implicit 'self'.
- ASSERT_TRUE(PrintedStmtObjCMatches(ObjCSource, returnStmt().bind("id"),
- "return ivar;\n",
- PolicyAdjusterType([](PrintingPolicy &PP) {
- PP.SuppressImplicitBase = true;
- })));
+ ASSERT_TRUE(PrintedStmtObjCMatches(
+ ObjCSource, returnStmt().bind("id"), "return ivar;\n",
+
+ [](PrintingPolicy &PP) { PP.SuppressImplicitBase = true; }));
// Print implicit 'self'.
ASSERT_TRUE(PrintedStmtObjCMatches(ObjCSource, returnStmt().bind("id"),
"return self->ivar;\n"));
@@ -243,5 +260,6 @@
// body not printed when TerseOutput is on.
ASSERT_TRUE(PrintedStmtCXXMatches(
StdVer::CXX11, CPPSource, lambdaExpr(anything()).bind("id"), "[] {}",
- PolicyAdjusterType([](PrintingPolicy &PP) { PP.TerseOutput = true; })));
+
+ [](PrintingPolicy &PP) { PP.TerseOutput = true; }));
}
Index: clang/unittests/AST/NamedDeclPrinterTest.cpp
===
--- clang/unittests/AST/NamedDeclPrinterTest.cpp
+++ clang/unittests/AST/NamedDeclPrinterTest.cpp
@@ -15,6 +15,7 @@
//
//===--===//
+#include "ASTPrint.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/Decl.h"
#include "clang/AST/PrettyPrinter.h"
@@ -66,31 +67,11 @@
const DeclarationMatcher &NodeMatch, StringRef ExpectedPrinted,
StringRef FileName,
std::function Print) {
- PrintMatch Printer(std::move(Print));
- MatchFinder Finder;
- Finder.addMatcher(NodeMatch
[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure
nridge added inline comments.
Comment at: clang/unittests/AST/DeclPrinterTest.cpp:59
+ return PrintedNodeMatches(Code, Args, NodeMatch, ExpectedPrinted,
+ FileName, PrinterType{PrintDecl},
+ PolicyModifier, AllowError);
dblaikie wrote:
> Is the `PrinterType{}` needed here, or could `PrintDecl` be passed
> directly/without explicitly constructing the wrapper? (functions should be
> implicitly convertible to the lambda type, I think?)
>
> Similarly for all the `PrintingPolicyAdjuster(...)` - might be able to skip
> that wrapping expression & rely on an implicit conversion.
Good call on both counts. (The `PrintingPolicyAdjuster(...)` stuff was left
over from when the typedef was for an `Optional` which you
correctly observed was redundant.)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105457/new/
https://reviews.llvm.org/D105457
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D104619: [clang] Respect PrintingPolicy::FullyQualifiedName when printing a template-id
nridge updated this revision to Diff 357827.
nridge added a comment.
Rebase on top of refactor patch changes
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104619/new/
https://reviews.llvm.org/D104619
Files:
clang/lib/AST/TypePrinter.cpp
clang/unittests/AST/CMakeLists.txt
clang/unittests/AST/TypePrinterTest.cpp
Index: clang/unittests/AST/TypePrinterTest.cpp
===
--- /dev/null
+++ clang/unittests/AST/TypePrinterTest.cpp
@@ -0,0 +1,65 @@
+//===- unittests/AST/TypePrinterTest.cpp --- Type printer tests
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file contains tests for QualType::print() and related methods.
+//
+//===--===//
+
+#include "ASTPrint.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/SmallString.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace ast_matchers;
+using namespace tooling;
+
+namespace {
+
+static void PrintType(raw_ostream &Out, const ASTContext *Context,
+ const QualType *T,
+ PrintingPolicyAdjuster PolicyAdjuster) {
+ assert(T && !T->isNull() && "Expected non-null Type");
+ PrintingPolicy Policy = Context->getPrintingPolicy();
+ if (PolicyAdjuster)
+PolicyAdjuster(Policy);
+ T->print(Out, Policy);
+}
+
+::testing::AssertionResult
+PrintedTypeMatches(StringRef Code, const std::vector &Args,
+ const DeclarationMatcher &NodeMatch,
+ StringRef ExpectedPrinted,
+ PrintingPolicyAdjuster PolicyAdjuster) {
+ return PrintedNodeMatches(Code, Args, NodeMatch, ExpectedPrinted,
+ "", PrintType, PolicyAdjuster);
+}
+
+} // unnamed namespace
+
+TEST(TypePrinter, TemplateId) {
+ std::string Code = R"cpp(
+namespace N {
+ template struct Type {};
+
+ template
+ void Foo(const Type &Param);
+}
+ )cpp";
+ auto Matcher = parmVarDecl(hasType(qualType().bind("id")));
+
+ ASSERT_TRUE(PrintedTypeMatches(
+ Code, {}, Matcher, "const Type &",
+ [](PrintingPolicy &Policy) { Policy.FullyQualifiedName = false; }));
+
+ ASSERT_TRUE(PrintedTypeMatches(
+ Code, {}, Matcher, "const N::Type &",
+ [](PrintingPolicy &Policy) { Policy.FullyQualifiedName = true; }));
+}
\ No newline at end of file
Index: clang/unittests/AST/CMakeLists.txt
===
--- clang/unittests/AST/CMakeLists.txt
+++ clang/unittests/AST/CMakeLists.txt
@@ -29,6 +29,7 @@
SourceLocationTest.cpp
StmtPrinterTest.cpp
StructuralEquivalenceTest.cpp
+ TypePrinterTest.cpp
)
clang_target_link_libraries(ASTTests
Index: clang/lib/AST/TypePrinter.cpp
===
--- clang/lib/AST/TypePrinter.cpp
+++ clang/lib/AST/TypePrinter.cpp
@@ -1456,7 +1456,7 @@
void TypePrinter::printTemplateSpecializationBefore(
const TemplateSpecializationType
*T,
raw_ostream &OS) {
- printTemplateId(T, OS, false);
+ printTemplateId(T, OS, Policy.FullyQualifiedName);
}
void TypePrinter::printTemplateSpecializationAfter(
Index: clang/unittests/AST/TypePrinterTest.cpp
===
--- /dev/null
+++ clang/unittests/AST/TypePrinterTest.cpp
@@ -0,0 +1,65 @@
+//===- unittests/AST/TypePrinterTest.cpp --- Type printer tests ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file contains tests for QualType::print() and related methods.
+//
+//===--===//
+
+#include "ASTPrint.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/SmallString.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace ast_matchers;
+using namespace tooling;
+
+namespace {
+
+static void PrintType(raw_ostream &Out, const ASTContext *Context,
+ const QualType *T,
+ PrintingPolicyAdjuster PolicyAdjuster) {
+ assert(T && !T->isNull() && "Expected non-null Type");
+ PrintingPolicy Policy = Context
[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness
jdoerfert reopened this revision. jdoerfert added a comment. Herald added a subscriber: dexonsmith. > A pthread_create() related test is XFAIL-ed, as it relied on it being > recognized as a builtin based on its name. > The builtin declaration syntax is too restrictive and doesn't allow custom > structs, function pointers, etc. > It seems to be the only case and fixing this would require reworking the > current builtin syntax, so this seems acceptable. First: Do I assume right this this feature was simply disabled without any plan to: - inform the authors (me) - update the documentation - re-enable support eventually or provide alternatives XFAILing a test and calling it a day seems inadequate IMHO. Second: Would an approach like this still work: https://reviews.llvm.org/D58531 ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77491/new/ https://reviews.llvm.org/D77491 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D105168: [RISCV] Unify the arch string parsing logic to RISCVISAInfo.
StephenFan added inline comments.
Comment at: llvm/lib/Support/RISCVISAInfo.cpp:115
+ return make_filter_range(SupportedExtensionInfos,
+ [&](const RISCVSupportedExtensionInfo &ExtInfo) {
+ return ExtInfo.Name == ExtName;
This lambda function will keep alive out of current scope.
Maybe the `[&]` should change to `[ExtName]`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105168/new/
https://reviews.llvm.org/D105168
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] d3e14fa - [analyzer][NFC] Display the correct function name even in crash dumps
Author: Balazs Benics
Date: 2021-07-12T09:06:46+02:00
New Revision: d3e14fafc69a07e3dab9ddb91f1d810bb5f8d7a0
URL:
https://github.com/llvm/llvm-project/commit/d3e14fafc69a07e3dab9ddb91f1d810bb5f8d7a0
DIFF:
https://github.com/llvm/llvm-project/commit/d3e14fafc69a07e3dab9ddb91f1d810bb5f8d7a0.diff
LOG: [analyzer][NFC] Display the correct function name even in crash dumps
The `-analyzer-display-progress` displayed the function name of the
currently analyzed function. It differs in C and C++. In C++, it
prints the argument types as well in a comma-separated list.
While in C, only the function name is displayed, without the brackets.
E.g.:
C++: foo(), foo(int, float)
C: foo
In crash traces, the analyzer dumps the location contexts, but the
string is not enough for `-analyze-function` in C++ mode.
This patch addresses the issue by dumping the proper function names
even in stack traces.
Reviewed By: NoQ
Differential Revision: https://reviews.llvm.org/D105708
Added:
Modified:
clang/include/clang/Analysis/AnalysisDeclContext.h
clang/lib/Analysis/AnalysisDeclContext.cpp
clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
clang/test/Analysis/crash-trace.c
Removed:
diff --git a/clang/include/clang/Analysis/AnalysisDeclContext.h
b/clang/include/clang/Analysis/AnalysisDeclContext.h
index d12582f4f329..102970a1d55e 100644
--- a/clang/include/clang/Analysis/AnalysisDeclContext.h
+++ b/clang/include/clang/Analysis/AnalysisDeclContext.h
@@ -200,6 +200,8 @@ class AnalysisDeclContext {
/// \returns Whether the root namespace of \p D is the \c std C++ namespace.
static bool isInStdNamespace(const Decl *D);
+ static std::string getFunctionName(const Decl *D);
+
private:
std::unique_ptr &getAnalysisImpl(const void *tag);
diff --git a/clang/lib/Analysis/AnalysisDeclContext.cpp
b/clang/lib/Analysis/AnalysisDeclContext.cpp
index 783de6442645..d8466ac34a3d 100644
--- a/clang/lib/Analysis/AnalysisDeclContext.cpp
+++ b/clang/lib/Analysis/AnalysisDeclContext.cpp
@@ -337,6 +337,59 @@ bool AnalysisDeclContext::isInStdNamespace(const Decl *D) {
return ND->isStdNamespace();
}
+std::string AnalysisDeclContext::getFunctionName(const Decl *D) {
+ std::string Str;
+ llvm::raw_string_ostream OS(Str);
+ const ASTContext &Ctx = D->getASTContext();
+
+ if (const FunctionDecl *FD = dyn_cast(D)) {
+OS << FD->getQualifiedNameAsString();
+
+// In C++, there are overloads.
+
+if (Ctx.getLangOpts().CPlusPlus) {
+ OS << '(';
+ for (const auto &P : FD->parameters()) {
+if (P != *FD->param_begin())
+ OS << ", ";
+OS << P->getType().getAsString();
+ }
+ OS << ')';
+}
+
+ } else if (isa(D)) {
+PresumedLoc Loc = Ctx.getSourceManager().getPresumedLoc(D->getLocation());
+
+if (Loc.isValid()) {
+ OS << "block (line: " << Loc.getLine() << ", col: " << Loc.getColumn()
+ << ')';
+}
+
+ } else if (const ObjCMethodDecl *OMD = dyn_cast(D)) {
+
+// FIXME: copy-pasted from CGDebugInfo.cpp.
+OS << (OMD->isInstanceMethod() ? '-' : '+') << '[';
+const DeclContext *DC = OMD->getDeclContext();
+if (const auto *OID = dyn_cast(DC)) {
+ OS << OID->getName();
+} else if (const auto *OID = dyn_cast(DC)) {
+ OS << OID->getName();
+} else if (const auto *OC = dyn_cast(DC)) {
+ if (OC->IsClassExtension()) {
+OS << OC->getClassInterface()->getName();
+ } else {
+OS << OC->getIdentifier()->getNameStart() << '('
+ << OC->getIdentifier()->getNameStart() << ')';
+ }
+} else if (const auto *OCD = dyn_cast(DC)) {
+ OS << OCD->getClassInterface()->getName() << '(' << OCD->getName() <<
')';
+}
+OS << ' ' << OMD->getSelector().getAsString() << ']';
+ }
+
+ return OS.str();
+}
+
LocationContextManager &AnalysisDeclContext::getLocationContextManager() {
assert(
ADCMgr &&
@@ -456,7 +509,7 @@ void LocationContext::dumpStack(raw_ostream &Out) const {
Out << "\t#" << Frame << ' ';
++Frame;
if (const auto *D = dyn_cast(LCtx->getDecl()))
-Out << "Calling " << D->getQualifiedNameAsString();
+Out << "Calling " << AnalysisDeclContext::getFunctionName(D);
else
Out << "Calling anonymous code";
if (const Stmt *S = cast(LCtx)->getCallSite()) {
diff --git a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
index 33914e98b90f..03b5c04f553f 100644
--- a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -209,8 +209,8 @@ class AnalysisConsumer : public AnalysisASTConsumer,
} else
assert(Mode == (AM_Syntax | AM_Path) && "Unexpected mode!");
- llvm::errs() << ": " << Loc.getFilename() << ' ' << getFunctionName(D)
- << '\n';
+ llv
[PATCH] D105708: [analyzer][NFC] Display the correct function name even in crash dumps
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd3e14fafc69a: [analyzer][NFC] Display the correct function
name even in crash dumps (authored by steakhal).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105708/new/
https://reviews.llvm.org/D105708
Files:
clang/include/clang/Analysis/AnalysisDeclContext.h
clang/lib/Analysis/AnalysisDeclContext.cpp
clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
clang/test/Analysis/crash-trace.c
Index: clang/test/Analysis/crash-trace.c
===
--- clang/test/Analysis/crash-trace.c
+++ clang/test/Analysis/crash-trace.c
@@ -1,4 +1,7 @@
-// RUN: not --crash %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection %s 2>&1 | FileCheck %s
+// RUN: not --crash %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection \
+// RUN: -x c %s 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-C-ONLY
+// RUN: not --crash %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection \
+// RUN: -x c++ %s 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-CXX-ONLY
// REQUIRES: crash-recovery
// Stack traces require back traces.
@@ -6,17 +9,22 @@
void clang_analyzer_crash(void);
-void inlined() {
+void inlined(int x, float y) {
clang_analyzer_crash();
}
void test() {
- inlined();
+ inlined(0, 0);
}
-// CHECK: 0. Program arguments: {{.*}}clang
-// CHECK-NEXT: 1. parser at end of file
-// CHECK-NEXT: 2. While analyzing stack:
-// CHECK-NEXT: #0 Calling inlined at line 14
-// CHECK-NEXT: #1 Calling test
-// CHECK-NEXT: 3. {{.*}}crash-trace.c:{{[0-9]+}}:3: Error evaluating statement
+// CHECK:0. Program arguments: {{.*}}clang
+// CHECK-NEXT: 1. parser at end of file
+// CHECK-NEXT: 2. While analyzing stack:
+//
+// CHECK-C-ONLY-NEXT: #0 Calling inlined at line 17
+// CHECK-C-ONLY-NEXT: #1 Calling test
+//
+// CHECK-CXX-ONLY-NEXT: #0 Calling inlined(int, float) at line 17
+// CHECK-CXX-ONLY-NEXT: #1 Calling test()
+//
+// CHECK-NEXT: 3. {{.*}}crash-trace.c:{{[0-9]+}}:3: Error evaluating statement
Index: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -209,8 +209,8 @@
} else
assert(Mode == (AM_Syntax | AM_Path) && "Unexpected mode!");
- llvm::errs() << ": " << Loc.getFilename() << ' ' << getFunctionName(D)
- << '\n';
+ llvm::errs() << ": " << Loc.getFilename() << ' '
+ << AnalysisDeclContext::getFunctionName(D) << '\n';
}
}
@@ -568,63 +568,10 @@
Mgr.reset();
}
-std::string AnalysisConsumer::getFunctionName(const Decl *D) {
- std::string Str;
- llvm::raw_string_ostream OS(Str);
-
- if (const FunctionDecl *FD = dyn_cast(D)) {
-OS << FD->getQualifiedNameAsString();
-
-// In C++, there are overloads.
-if (Ctx->getLangOpts().CPlusPlus) {
- OS << '(';
- for (const auto &P : FD->parameters()) {
-if (P != *FD->param_begin())
- OS << ", ";
-OS << P->getType().getAsString();
- }
- OS << ')';
-}
-
- } else if (isa(D)) {
-PresumedLoc Loc = Ctx->getSourceManager().getPresumedLoc(D->getLocation());
-
-if (Loc.isValid()) {
- OS << "block (line: " << Loc.getLine() << ", col: " << Loc.getColumn()
- << ')';
-}
-
- } else if (const ObjCMethodDecl *OMD = dyn_cast(D)) {
-
-// FIXME: copy-pasted from CGDebugInfo.cpp.
-OS << (OMD->isInstanceMethod() ? '-' : '+') << '[';
-const DeclContext *DC = OMD->getDeclContext();
-if (const auto *OID = dyn_cast(DC)) {
- OS << OID->getName();
-} else if (const auto *OID = dyn_cast(DC)) {
- OS << OID->getName();
-} else if (const auto *OC = dyn_cast(DC)) {
- if (OC->IsClassExtension()) {
-OS << OC->getClassInterface()->getName();
- } else {
-OS << OC->getIdentifier()->getNameStart() << '('
- << OC->getIdentifier()->getNameStart() << ')';
- }
-} else if (const auto *OCD = dyn_cast(DC)) {
- OS << OCD->getClassInterface()->getName() << '('
- << OCD->getName() << ')';
-}
-OS << ' ' << OMD->getSelector().getAsString() << ']';
-
- }
-
- return OS.str();
-}
-
AnalysisConsumer::AnalysisMode
AnalysisConsumer::getModeForDecl(Decl *D, AnalysisMode Mode) {
if (!Opts->AnalyzeSpecificFunction.empty() &&
- getFunctionName(D) != Opts->AnalyzeSpecificFunction)
+ AnalysisDeclContext::getFunctionName(D) != Opts->AnalyzeSpecificFunction)
return AM_None;
// Unless -analyze-all is specified, treat decls differently depending on
Index: clang/lib/Analysis/AnalysisDeclContext.cpp
===
--- clang/lib/Analysis/
