Related issue: https://github.com/llvm/llvm-project/issues/56246
|2,020 ms||x64 debian > ThreadSanitizer-x86_64.ThreadSanitizer-x86_64::fd_location_closed.cpp|
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang --driver-mode=g++ -fsanitize=thread -Wall -m64 -msse4.2 -gline-tables-only -I/var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/tsan/../ -std=c++11 -I/var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/tsan/../ -O1 /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/tsan/fd_location_closed.cpp -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/tsan/X86_64Config/Output/fd_location_closed.cpp.tmp && /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/tsan/deflake.bash 10 /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/tsan/X86_64Config/Output/fd_location_closed.cpp.tmp |& FileCheck /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/tsan/fd_location_closed.cpp
Thanks for this! Can you add some more details to the patch description so it's more clear from there why the patch is necessary? Also, can you add a release note for the fix?
I'm pretty sure the changes here are correct, but I want to take another run at the standards wording to be sure.
Add the docs?
You should add a new line back to the end of the file.
When not in a constant-context, what should we be doing here? Why doesn't that set the variable?
Will this visit end up looking into OTHER things here? I guess I'm concerned about something like:
this->get_some_other_type().static_func() and us skipping the this-> for THAT, despite it not being a static call in that context.
Given this has not made progress in a while, I think we should try to implement https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2280r4.html instead of this approach, as I'm not sure if that's strictly conforming, and a more holistic approach is likely to lead to better results.
What do people think?
P2280R4 was adopted for C++23, so I think we'll need to do the work eventually. It likely makes sense to explore doing that work now, at the very least so we don't make it harder to implement in the future.