Page MenuHomePhabricator

Fix emission of consteval constructor of derived type
Needs ReviewPublic

Authored by Fznamznon on Wed, Jan 25, 4:42 AM.

Details

Summary

For simple derived type ConstantEmitter returns a struct of the same
size but different type which is then stored field-by-field into memory
via pointer to derived type. In case base type has more fields than derived,
the incorrect GEP is emitted. So, just cast pointer to derived type to
appropriate type with enough fields.

Fixes #60166

Diff Detail

Unit TestsFailed

TimeTest
6,740 mslibcxx CI C++2b > llvm-libc++-shared-cfg-in.libcxx/gdb::gdb_pretty_printer_test.sh.cpp
Script: -- : 'RUN: at line 20'; /home/libcxx-builder/.buildkite-agent/builds/68de55d53707-1/llvm-project/libcxx-ci/install/bin/clang++ --target=x86_64-unknown-linux-gnu /home/libcxx-builder/.buildkite-agent/builds/68de55d53707-1/llvm-project/libcxx-ci/libcxx/test/libcxx/gdb/gdb_pretty_printer_test.sh.cpp -o /home/libcxx-builder/.buildkite-agent/builds/68de55d53707-1/llvm-project/libcxx-ci/build/generic-cxx2b/test/libcxx/gdb/Output/gdb_pretty_printer_test.sh.cpp.dir/t.tmp.exe -nostdinc++ -I /home/libcxx-builder/.buildkite-agent/builds/68de55d53707-1/llvm-project/libcxx-ci/build/generic-cxx2b/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/68de55d53707-1/llvm-project/libcxx-ci/build/generic-cxx2b/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/68de55d53707-1/llvm-project/libcxx-ci/libcxx/test/support -std=c++2b -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-noexcept-type -Wno-atomic-alignment -Wno-user-defined-literals -Wno-tautological-compare -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_ENABLE_EXPERIMENTAL -D_LIBCPP_DISABLE_AVAILABILITY -Werror=thread-safety -Wuser-defined-warnings -g -lc++experimental -nostdlib++ -L /home/libcxx-builder/.buildkite-agent/builds/68de55d53707-1/llvm-project/libcxx-ci/build/generic-cxx2b/lib -Wl,-rpath,/home/libcxx-builder/.buildkite-agent/builds/68de55d53707-1/llvm-project/libcxx-ci/build/generic-cxx2b/lib -lc++ -pthread
3,750 mslibcxx CI Modules > llvm-libc++-shared-cfg-in.libcxx/algorithms/specialized_algorithms/special_mem_concepts::nothrow_sentinel_for.compile.pass.cpp
Script: -- : 'COMPILED WITH'; /home/libcxx-builder/.buildkite-agent/builds/2107af466208-1/llvm-project/libcxx-ci/install/bin/clang++ /home/libcxx-builder/.buildkite-agent/builds/2107af466208-1/llvm-project/libcxx-ci/libcxx/test/libcxx/algorithms/specialized.algorithms/special.mem.concepts/nothrow_sentinel_for.compile.pass.cpp --target=x86_64-unknown-linux-gnu -nostdinc++ -I /home/libcxx-builder/.buildkite-agent/builds/2107af466208-1/llvm-project/libcxx-ci/build/generic-modules/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/2107af466208-1/llvm-project/libcxx-ci/build/generic-modules/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/2107af466208-1/llvm-project/libcxx-ci/libcxx/test/support -std=c++2b -fmodules -fcxx-modules -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-noexcept-type -Wno-atomic-alignment -Wno-user-defined-literals -Wno-tautological-compare -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_ENABLE_EXPERIMENTAL -D_LIBCPP_DISABLE_AVAILABILITY -Werror=thread-safety -Wuser-defined-warnings -fsyntax-only
4,430 mslibcxx CI Modules > llvm-libc++-shared-cfg-in.libcxx/gdb::gdb_pretty_printer_test.sh.cpp
Script: -- : 'RUN: at line 20'; /home/libcxx-builder/.buildkite-agent/builds/2107af466208-1/llvm-project/libcxx-ci/install/bin/clang++ --target=x86_64-unknown-linux-gnu /home/libcxx-builder/.buildkite-agent/builds/2107af466208-1/llvm-project/libcxx-ci/libcxx/test/libcxx/gdb/gdb_pretty_printer_test.sh.cpp -o /home/libcxx-builder/.buildkite-agent/builds/2107af466208-1/llvm-project/libcxx-ci/build/generic-modules/test/libcxx/gdb/Output/gdb_pretty_printer_test.sh.cpp.dir/t.tmp.exe -nostdinc++ -I /home/libcxx-builder/.buildkite-agent/builds/2107af466208-1/llvm-project/libcxx-ci/build/generic-modules/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/2107af466208-1/llvm-project/libcxx-ci/build/generic-modules/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/2107af466208-1/llvm-project/libcxx-ci/libcxx/test/support -std=c++2b -fmodules -fcxx-modules -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-noexcept-type -Wno-atomic-alignment -Wno-user-defined-literals -Wno-tautological-compare -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_ENABLE_EXPERIMENTAL -D_LIBCPP_DISABLE_AVAILABILITY -Werror=thread-safety -Wuser-defined-warnings -g -lc++experimental -nostdlib++ -L /home/libcxx-builder/.buildkite-agent/builds/2107af466208-1/llvm-project/libcxx-ci/build/generic-modules/lib -Wl,-rpath,/home/libcxx-builder/.buildkite-agent/builds/2107af466208-1/llvm-project/libcxx-ci/build/generic-modules/lib -lc++ -pthread
1,540 msx64 debian > XRay-x86_64-linux.TestCases/Posix::fdr-mode.cpp
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang --driver-mode=g++ -fxray-instrument -m64 -g -std=c++11 /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/xray/TestCases/Posix/fdr-mode.cpp -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/xray/X86_64LinuxConfig/TestCases/Posix/Output/fdr-mode.cpp.tmp

Event Timeline

Fznamznon created this revision.Wed, Jan 25, 4:42 AM
Herald added a project: Restricted Project. · View Herald TranscriptWed, Jan 25, 4:42 AM
Fznamznon requested review of this revision.Wed, Jan 25, 4:42 AM
Herald added a project: Restricted Project. · View Herald TranscriptWed, Jan 25, 4:42 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

Unit tests fail seems unrelated. I'm seeing the same test fail for a commit that is already in main https://github.com/llvm/llvm-project/commit/f1f583347d00aad378eb0128e72d3d2e8be5174b .

shafik added a subscriber: shafik.Wed, Jan 25, 7:52 AM

Makes sense to me but I am not familiar with this area so I will let someone else who has more knowledge approve.

clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
95

nitpick: I have been using GH as a prefix for Github issues.

118

I think having a link to the github issue in the summary allows for the issue be closed automatically when you commit. Is this correct @aaron.ballman I have been doing this for a while and they get closed when I commit but maybe there is another mechanism involved.

I think having a link to the github issue in the summary allows for the issue be closed automatically when you commit. Is this correct @aaron.ballman I have been doing this for a while and they get closed when I commit but maybe there is another mechanism involved.

This works at least for GitHub pull requests, see https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue . We've been using this in a downstream project. I was hoping it works in the same way for commits, so added issue ID to a commit message.

Fznamznon updated this revision to Diff 492485.Thu, Jan 26, 9:23 AM

Rebase and apply nit

I think having a link to the github issue in the summary allows for the issue be closed automatically when you commit. Is this correct @aaron.ballman I have been doing this for a while and they get closed when I commit but maybe there is another mechanism involved.

This works at least for GitHub pull requests, see https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue . We've been using this in a downstream project. I was hoping it works in the same way for commits, so added issue ID to a commit message.

Yes, this works the same way for commits here. I learned recently that folks appreciate a full link to the issue (Fixes https://github.com/llvm/llvm-project/issues/NNNN) instead of Fixes #NNNN because it makes it easier to traverse from the commit message to the github issue (and it will still automatically close the issue on commit), so you might consider changing that when you land the patch.

The changes look correct to me, but codegen is not my area of expertise, so adding some other codegen reviewers to double-check.