[clang] Reland "[Modules] Record whether VarDecl initializers contain side effects" (PR #145447)
ilya-biryukov wrote:
Here's a small repro:
```cpp
export module Foo;
export template
struct Wrapper {
double value;
};
export constexpr Wrapper Compute() {
return Wrapper{1.0};
}
export template
Wrapper ComputeInFloat() {
const Wrapper a = Compute();
return a;
}
```
it fails when assertions are on: https://gcc.godbolt.org/z/vsW69z9Pd
The problem is that type of the `VarDecl` itself is dependent (the dependent
`TemplateSpecializationType`) and so `castAs` fails.
My gut feeling is that the right fix would be to avoid running
`CheckEvaluationResult` if `VarDecl` has a dependent type altogether and leave
this diagnostic to template instantiations.
https://github.com/llvm/llvm-project/pull/145447
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland "[Modules] Record whether VarDecl initializers contain side effects" (PR #145447)
hnrklssn wrote: > > You don't happen to be using the new constant interpreter? > > Not sure what you mean by this. We're using the unmodified compiler and > testing its correctness on google internal code. Then you're likely not. I was just making sure, because the constant evaluation takes a different path if the new (experimental) constant interpreter is enabled. https://github.com/llvm/llvm-project/pull/145447 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland "[Modules] Record whether VarDecl initializers contain side effects" (PR #145447)
bgra8 wrote: > Do you have a reproducer you can share? Working to get a smaller repro. But this only reproduces for builds with modules enabled so it will take some time. > Does the type that's being casted to RecordType have RecordType as the > canonical type, or is it completely off? The compiler seems to choke on serializing a const member initialized with a value of the same type as the member (but the type is the result of multiple macro expansions and I didn't have yet the time to investigate thoroughly) > You don't happen to be using the new constant interpreter? Not sure what you mean by this. We're using the unmodified compiler and testing its correctness on google internal code. https://github.com/llvm/llvm-project/pull/145447 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland "[Modules] Record whether VarDecl initializers contain side effects" (PR #145447)
hnrklssn wrote: > I patched #146468 after this change, rebuilt the compiler and reproduced the > same crash. So I guess that patch does not fix the issue I see here. Does > #146468 depend on some other change? It does not. Do you have a reproducer you can share? Does the type that's being casted to `RecordType` have `RecordType` as the canonical type, or is it completely off? Perhaps it could be as simple as replacing `castAs` with `getAs`..? You don't happen to be using the new constant interpreter? https://github.com/llvm/llvm-project/pull/145447 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland "[Modules] Record whether VarDecl initializers contain side effects" (PR #145447)
bgra8 wrote: > Is it fixed by https://github.com/llvm/llvm-project/pull/146468? I patched https://github.com/llvm/llvm-project/pull/146468 after this change, rebuilt the compiler and reproduced the same crash. So I guess that patch does not fix the issue I see here. Does https://github.com/llvm/llvm-project/pull/146468 depend on some other change? The assertion looks similar: ``` assert.h assertion failed at llvm-project/llvm/include/llvm/Support/Casting.h:566 in decltype(auto) llvm::cast(const From &) [To = clang::RecordType, From = clang::QualType]: isa(Val) && "cast() argument of incompatible type!" *** Check failure stack trace: *** @ 0x564b4bf20f04 absl::log_internal::LogMessage::SendToLog() @ 0x564b4bf20ec7 absl::log_internal::LogMessage::Flush() @ 0x564b4c149204 __assert_fail @ 0x564b48372ad0 CheckEvaluationResult() @ 0x564b483353a5 clang::Expr::EvaluateAsInitializer() @ 0x564b4824eb32 clang::VarDecl::evaluateValueImpl() @ 0x564b4824e07a clang::VarDecl::evaluateValue() @ 0x564b4824df78 clang::VarDecl::hasInitWithSideEffects() @ 0x564b47395839 clang::ASTDeclWriter::VisitVarDecl() @ 0x564b4738e996 clang::ASTDeclWriter::Visit() @ 0x564b473a18bc clang::ASTWriter::WriteDecl() @ 0x564b47351725 clang::ASTWriter::WriteDeclAndTypes() @ 0x564b4734b42b clang::ASTWriter::WriteASTCore() @ 0x564b4734a131 clang::ASTWriter::WriteAST() @ 0x564b473bb1e0 clang::PCHGenerator::HandleTranslationUnit() @ 0x564b46f27cfd clang::MultiplexConsumer::HandleTranslationUnit() @ 0x564b4713a948 clang::ParseAST() @ 0x564b46e6e34a clang::FrontendAction::Execute() @ 0x564b46de125d clang::CompilerInstance::ExecuteAction() @ 0x564b4622586b clang::ExecuteCompilerInvocation() @ 0x564b46219218 cc1_main() @ 0x564b46216334 ExecuteCC1Tool() @ 0x564b46f97a3e llvm::function_ref<>::callback_fn<>() @ 0x564b4bd95c9c llvm::CrashRecoveryContext::RunSafely() @ 0x564b46f96f24 clang::driver::CC1Command::Execute() @ 0x564b46f57615 clang::driver::Compilation::ExecuteCommand() @ 0x564b46f578af clang::driver::Compilation::ExecuteJobs() @ 0x564b46f73200 clang::driver::Driver::ExecuteCompilation() @ 0x564b4621593d clang_main() @ 0x564b46213cf4 main @ 0x7f816c5b43d4 __libc_start_main @ 0x564b46213c2a _start ``` https://github.com/llvm/llvm-project/pull/145447 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland "[Modules] Record whether VarDecl initializers contain side effects" (PR #145447)
hnrklssn wrote: > @hnrklssn we (at google) have bisected a clang crash at the original commit > (https://github.com/llvm/llvm-project/commit/319a51a5ffb807b88ae7f73676894bf306a0d134) > that also reproduces at this revision. > > > > Here's the stack trace: > > > > ``` > > assert.h assertion failed at > llvm-project/llvm/include/llvm/Support/Casting.h:566 in decltype(auto) > llvm::cast(const From &) [To = clang::RecordType, >From = clang::QualType]: > isa(Val) && "cast() argument of incompatible type!" > > *** Check failure stack trace: *** > > @ 0x55b57e120f04 absl::log_internal::LogMessage::SendToLog() > > @ 0x55b57e120ec7 absl::log_internal::LogMessage::Flush() > > @ 0x55b57e349204 __assert_fail > > @ 0x55b57a572ad0 CheckEvaluationResult() > > @ 0x55b57a5353a5 clang::Expr::EvaluateAsInitializer() > > @ 0x55b57a44eb32 clang::VarDecl::evaluateValueImpl() > > @ 0x55b57a44e07a clang::VarDecl::evaluateValue() > > @ 0x55b57a44df78 clang::VarDecl::hasInitWithSideEffects() > > @ 0x55b579595839 clang::ASTDeclWriter::VisitVarDecl() > > @ 0x55b57958e996 clang::ASTDeclWriter::Visit() > > @ 0x55b5795a18bc clang::ASTWriter::WriteDecl() > > @ 0x55b579551725 clang::ASTWriter::WriteDeclAndTypes() > > @ 0x55b57954b42b clang::ASTWriter::WriteASTCore() > > @ 0x55b57954a131 clang::ASTWriter::WriteAST() > > @ 0x55b5795bb1e0 clang::PCHGenerator::HandleTranslationUnit() > > @ 0x55b579127cfd clang::MultiplexConsumer::HandleTranslationUnit() > > @ 0x55b57933a948 clang::ParseAST() > > @ 0x55b57906e34a clang::FrontendAction::Execute() > > @ 0x55b578fe125d clang::CompilerInstance::ExecuteAction() > > @ 0x55b57842586b clang::ExecuteCompilerInvocation() > > @ 0x55b578419218 cc1_main() > > @ 0x55b578416334 ExecuteCC1Tool() > > @ 0x55b579197a3e llvm::function_ref<>::callback_fn<>() > > @ 0x55b57df95c9c llvm::CrashRecoveryContext::RunSafely() > > @ 0x55b579196f24 clang::driver::CC1Command::Execute() > > @ 0x55b579157615 clang::driver::Compilation::ExecuteCommand() > > @ 0x55b5791578af clang::driver::Compilation::ExecuteJobs() > > @ 0x55b579173200 clang::driver::Driver::ExecuteCompilation() > > @ 0x55b57841593d clang_main() > > @ 0x55b578413cf4 main > > @ 0x7fc1572933d4 __libc_start_main > > @ 0x55b578413c2a _start > > ``` > > > > Can you please take a look? Is it fixed by https://github.com/llvm/llvm-project/pull/146468? https://github.com/llvm/llvm-project/pull/145447 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland "[Modules] Record whether VarDecl initializers contain side effects" (PR #145447)
bgra8 wrote: @hnrklssn we (at google) have bisected a clang crash at the original commit (https://github.com/llvm/llvm-project/commit/f2e9f882e07e0b6507ece9ca8a556cd5c659) that also reproduces at this revision. Here's the stack trace: ``` assert.h assertion failed at llvm-project/llvm/include/llvm/Support/Casting.h:566 in decltype(auto) llvm::cast(const From &) [To = clang::RecordType, From = clang::QualType]: isa(Val) && "cast() argument of incompatible type!" *** Check failure stack trace: *** @ 0x55b57e120f04 absl::log_internal::LogMessage::SendToLog() @ 0x55b57e120ec7 absl::log_internal::LogMessage::Flush() @ 0x55b57e349204 __assert_fail @ 0x55b57a572ad0 CheckEvaluationResult() @ 0x55b57a5353a5 clang::Expr::EvaluateAsInitializer() @ 0x55b57a44eb32 clang::VarDecl::evaluateValueImpl() @ 0x55b57a44e07a clang::VarDecl::evaluateValue() @ 0x55b57a44df78 clang::VarDecl::hasInitWithSideEffects() @ 0x55b579595839 clang::ASTDeclWriter::VisitVarDecl() @ 0x55b57958e996 clang::ASTDeclWriter::Visit() @ 0x55b5795a18bc clang::ASTWriter::WriteDecl() @ 0x55b579551725 clang::ASTWriter::WriteDeclAndTypes() @ 0x55b57954b42b clang::ASTWriter::WriteASTCore() @ 0x55b57954a131 clang::ASTWriter::WriteAST() @ 0x55b5795bb1e0 clang::PCHGenerator::HandleTranslationUnit() @ 0x55b579127cfd clang::MultiplexConsumer::HandleTranslationUnit() @ 0x55b57933a948 clang::ParseAST() @ 0x55b57906e34a clang::FrontendAction::Execute() @ 0x55b578fe125d clang::CompilerInstance::ExecuteAction() @ 0x55b57842586b clang::ExecuteCompilerInvocation() @ 0x55b578419218 cc1_main() @ 0x55b578416334 ExecuteCC1Tool() @ 0x55b579197a3e llvm::function_ref<>::callback_fn<>() @ 0x55b57df95c9c llvm::CrashRecoveryContext::RunSafely() @ 0x55b579196f24 clang::driver::CC1Command::Execute() @ 0x55b579157615 clang::driver::Compilation::ExecuteCommand() @ 0x55b5791578af clang::driver::Compilation::ExecuteJobs() @ 0x55b579173200 clang::driver::Driver::ExecuteCompilation() @ 0x55b57841593d clang_main() @ 0x55b578413cf4 main @ 0x7fc1572933d4 __libc_start_main @ 0x55b578413c2a _start ``` Can you please take a look? https://github.com/llvm/llvm-project/pull/145447 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland "[Modules] Record whether VarDecl initializers contain side effects" (PR #145447)
@@ -16719,6 +16719,9 @@ static bool EvaluateInPlace(APValue &Result, EvalInfo
&Info, const LValue &This,
const Expr *E, bool AllowNonLiteralTypes) {
assert(!E->isValueDependent());
+ if (E->getType().isNull())
ChuanqiXu9 wrote:
Let's add a comment for what you write in the commit message.
https://github.com/llvm/llvm-project/pull/145447
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland "[Modules] Record whether VarDecl initializers contain side effects" (PR #145447)
hnrklssn wrote: > What is the difference between this one and the previous PR? The second commit, where I prevent constant evaluation if the expression type is null https://github.com/llvm/llvm-project/pull/145447 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland "[Modules] Record whether VarDecl initializers contain side effects" (PR #145447)
https://github.com/ChuanqiXu9 approved this pull request. https://github.com/llvm/llvm-project/pull/145447 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland "[Modules] Record whether VarDecl initializers contain side effects" (PR #145447)
llvm-ci wrote:
LLVM Buildbot has detected a new failure on builder `clang-m68k-linux-cross`
running on `suse-gary-m68k-cross` while building `clang` at step 5 "ninja check
1".
Full details are available at:
https://lab.llvm.org/buildbot/#/builders/27/builds/12004
Here is the relevant piece of the build log for the reference
```
Step 5 (ninja check 1) failure: stage 1 checked (failure)
...
In file included from
/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang/include/clang/Parse/Parser.h:20,
from
/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang/unittests/Analysis/MacroExpansionContextTest.cpp:23:
/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang/include/clang/Sema/Sema.h:839:7:
warning: ‘clang::Sema’ declared with greater visibility than the type of its
field ‘clang::Sema::UnusedFileScopedDecls’ [-Wattributes]
839 | class Sema final : public SemaBase {
| ^~~~
/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang/include/clang/Sema/Sema.h:839:7:
warning: ‘clang::Sema’ declared with greater visibility than the type of its
field ‘clang::Sema::TentativeDefinitions’ [-Wattributes]
/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang/include/clang/Sema/Sema.h:839:7:
warning: ‘clang::Sema’ declared with greater visibility than the type of its
field ‘clang::Sema::ExtVectorDecls’ [-Wattributes]
/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang/include/clang/Sema/Sema.h:839:7:
warning: ‘clang::Sema’ declared with greater visibility than the type of its
field ‘clang::Sema::DelegatingCtorDecls’ [-Wattributes]
[123/377] Building CXX object
tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/StaticAnalyzer/AnalyzerOptionsTest.cpp.o
[124/377] Building CXX object
tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/IntervalPartitionTest.cpp.o
FAILED:
tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/IntervalPartitionTest.cpp.o
/usr/bin/c++ -DGTEST_HAS_RTTI=0 -DLLVM_BUILD_STATIC -D_DEBUG
-D_GLIBCXX_ASSERTIONS -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS
-D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS
-I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/stage1/tools/clang/unittests
-I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang/unittests
-I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang/include
-I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/stage1/tools/clang/include
-I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/stage1/include
-I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/llvm/include
-I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang/unittests/Tooling
-I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/third-party/unittest/googletest/include
-I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/third-party/unittest/googlemock/include
-fPIC -fno-semantic-interposition -fvisibility-inlines-hidden
-Werror=date-time -Wall -Wextra -Wno-unused-parameter -Wwrite-strings
-Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long
-Wimplicit-fallthrough -Wno-maybe-uninitialized -Wno-nonnull
-Wno-class-memaccess -Wno-redundant-move -Wno-pessimizing-move
-Wno-noexcept-type -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment
-Wno-misleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color
-ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -O3
-DNDEBUG -std=c++17 -Wno-variadic-macros -fno-exceptions -funwind-tables
-fno-rtti -UNDEBUG -Wno-suggest-override -MD -MT
tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/IntervalPartitionTest.cpp.o
-MF
tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/IntervalPartitionTest.cpp.o.d
-o
tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/IntervalPartitionTest.cpp.o
-c
/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang/unittests/Analysis/IntervalPartitionTest.cpp
c++: fatal error: Killed signal terminated program cc1plus
compilation terminated.
[125/377] Building CXX object
tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/StaticAnalyzer/APSIntTypeTest.cpp.o
[126/377] Building CXX object
tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/ASTOpsTest.cpp.o
[127/377] Building CXX object
tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp.o
[128/377] Building CXX object
tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/DeterminismTest.cpp.o
[129/377] Building CXX object
tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/Logg
[clang] Reland "[Modules] Record whether VarDecl initializers contain side effects" (PR #145447)
https://github.com/ChuanqiXu9 commented: What is the difference between this one and the previous PR? https://github.com/llvm/llvm-project/pull/145447 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
