Differential D129973
[clang] Pass FoundDecl to DeclRefExpr creator for operator overloads Authored by SimplyDanny on Jul 17 2022, 12:29 PM.
Details Without the "found declaration" it is later not possible to know where the operator declaration The initial motivation for this fix came from #55095. However, this also has an influence on
Diff Detail
Unit Tests Event TimelineComment Actions Thank you for working on this. It looks like there are some pre-commit test failures, did you confirm whether they were related to your change or not? Also you mentioned that this changes the output of -ast-dump it might be worth adding a test to capture this behavior change.
Comment Actions LGTM
Comment Actions Thank you for the review! Before I push the change into main I may add an entry to clang/docs/ReleaseNotes.rst, right? And do you agree that the test failures are unrelated to my change? It looks like they are all located in libomptarget caused by a linker error. Comment Actions Yes, please add an entry in the release notes.
They don't look related but as always you should be ready to revert if any of the builds break in a way that you can't fix quickly. Comment Actions With this change we don't pass "LocInfo" directly and it seems to break the locations when calling "getCXXOperatorNameRange" for this DeclRefExpr later on. Please fix it. You can introduce another "Create" static method for DeclRefExpr that accepts LocInfo and passes it to the DeclarationNameInfo constructor. Comment Actions Thank you for the hint! This indeed "fixes" the test references so that they can be reverted to their original state. This is done in D130799. Instead of a new create method or a new constructor (there are already quite a lot of them with complex signatures), I created a DeclarationNameInfo and passed it to one of the existing creators. Comment Actions Heads up: This commit causes clang crashes on our end. We are currently working on a reproducer and will post it as soon as its ready. Comment Actions Here is the reduced code: template <class a> struct b {
template <class c> void ab(c);
template <class c> void ad(c p1) { ab(d()(p1)); }
a d();
};
template <typename e> struct f {
using aa = e;
template <class g> struct j {
g ag;
template <class... h> auto operator()(h... p1) -> decltype(ag(p1...));
};
using ai = b<j<aa>>;
template <class c> void ad(c p1) { i.ad(p1); }
ai i;
};
template <typename c, typename> struct l {
int k(const c &);
struct n {
int operator()(const c &) const;
};
using m = f<n>;
m aj;
};
template <typename c, typename al> int l<c, al>::k(const c &p1) { aj.ad(p1); }
template <typename c, typename al>
int l<c, al>::n::operator()(const c &) const {}
template class l<int, int>;This commit causes a compiler crash for this code. It however successfully builds with the previous commit compiler. Please revert! Comment Actions This is also causing a compilation error in some google third-party code. Compiling with older clang doesn't give that error. Error: error: no matching function for call to object of type 'key_compare_adapter<QuicIntervalSet<long>::IntervalLess>::checked_compare'
comp(__trans_tmp_3, k);
^~~~
/tmp/pre.ii:85:5: note: in instantiation of function template specialization 'btree_node<set_params<QuicIntervalSet<long>::IntervalLess>>::linear_search_impl<int, key_compare_adapter<QuicIntervalSet<long>::IntervalLess>::checked_compare>' requested here
linear_search_impl(k, 0, 0, comp,
^
/tmp/pre.ii:76:32: note: in instantiation of function template specialization 'btree_node<set_params<QuicIntervalSet<long>::IntervalLess>>::linear_search<int, key_compare_adapter<QuicIntervalSet<long>::IntervalLess>::checked_compare>' requested here
use_linear_search::value ? linear_search(k, comp) : binary_search(k, comp);
^
/tmp/pre.ii:123:15: note: in instantiation of function template specialization 'btree_node<set_params<QuicIntervalSet<long>::IntervalLess>>::lower_bound<int>' requested here
iter.node_->lower_bound(key, key_comp());
^
/tmp/pre.ii:111:51: note: in instantiation of function template specialization 'btree<set_params<QuicIntervalSet<long>::IntervalLess>>::internal_lower_bound<int>' requested here
template <typename K> void lower_bound(K key) { internal_lower_bound(key); }
^
/tmp/pre.ii:133:37: note: in instantiation of function template specialization 'btree<set_params<QuicIntervalSet<long>::IntervalLess>>::lower_bound<int>' requested here
void lower_bound(int key) { tree_.lower_bound(key); }
^
/tmp/pre.ii:164:14: note: in instantiation of member function 'btree_container<btree<set_params<QuicIntervalSet<long>::IntervalLess>>>::lower_bound' requested here
intervals_.lower_bound(0);
^
/tmp/pre.ii:158:16: note: in instantiation of member function 'QuicIntervalSet<long>::Add' requested here
void Add() { Add(value_type(Add_min, Add_max)); }
^
/tmp/pre.ii:190:34: note: in instantiation of member function 'QuicIntervalSet<long>::Add' requested here
unacked_frame_headers_offsets_.Add();
^
/tmp/pre.ii:44:10: note: candidate template ignored: substitution failure [with T = QuicInterval, U = int]
void operator()(T lhs, U rhs) {I attached the preprocessed file for the case .The commande to reproduce is: clang -msse4.2 -O1 -fsized-deallocation -std=gnu++17 -c pre.ii Comment Actions @SimplyDanny given the reports from @joanahalili and @asmok-g are you considering a revert for this patch? Comment Actions @tstellar FYI the clang crash report from @joanahalili and the new compilation errors report from @asmok-g, both introduced by this patch. Comment Actions Reverting for @bgraur as he's having some issues with his commit access ATM. To sum up the reproducer attached by @joanahalili triggers a crash https://reviews.llvm.org/rG4e94f6653150511de434fa7e29b684ae7f0e52b6 with stack trace and error: $ ~/repos/llvm/build/bin/clang -xc++ -c a.cc a.cc:3:38: error: cannot compile this l-value expression yet template <class c> void ad(c p1) { ab(d()(p1)); } ^~ Unexpected placeholder builtin type! UNREACHABLE executed at /usr/local/google/home/kadircet/repos/llvm/clang/lib/CodeGen/CodeGenTypes.cpp:641! PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script. Stack dump: 0. Program arguments: /usr/local/google/home/kadircet/repos/llvm/build/bin/clang -xc++ -c a.cc 1. <eof> parser at end of file 2. Per-file LLVM IR generation 3. a.cc:3:27: Generating code for declaration 'b<f<l<int, int>::n>::j<l<int, int>::n>>::ad' #0 0x00000000064fb563 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /usr/local/google/home/kadircet/repos/llvm/llvm/lib/Support/Unix/Signals.inc:569:13 #1 0x00000000064f92fe llvm::sys::RunSignalHandlers() /usr/local/google/home/kadircet/repos/llvm/llvm/lib/Support/Signals.cpp:104:18 #2 0x000000000645f823 (anonymous namespace)::CrashRecoveryContextImpl::HandleCrash(int, unsigned long) /usr/local/google/home/kadircet/repos/llvm/llvm/lib/Support/CrashRecoveryContext.cpp:76:5 #3 0x000000000645f9de CrashRecoverySignalHandler(int) /usr/local/google/home/kadircet/repos/llvm/llvm/lib/Support/CrashRecoveryContext.cpp:390:1 #4 0x00007f8c235bf200 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x12200) #5 0x00007f8c2302c8a1 raise ./signal/../sysdeps/unix/sysv/linux/raise.c:50:1 #6 0x00007f8c23016546 abort ./stdlib/abort.c:81:7 #7 0x0000000006465531 (/usr/local/google/home/kadircet/repos/llvm/build/bin/clang+0x6465531) #8 0x000000000689b09f (/usr/local/google/home/kadircet/repos/llvm/build/bin/clang+0x689b09f) #9 0x00000000068be0f1 clang::CodeGen::CodeGenFunction::EmitUnsupportedLValue(clang::Expr const*, char const*) /usr/local/google/home/kadircet/repos/llvm/clang/lib/CodeGen/CGExpr.cpp:1250:22 #10 0x00000000068b4703 clang::CodeGen::CodeGenFunction::EmitLValue(clang::Expr const*) /usr/local/google/home/kadircet/repos/llvm/clang/lib/CodeGen/CGExpr.cpp:0:19 #11 0x00000000068d3fab isSimple /usr/local/google/home/kadircet/repos/llvm/clang/lib/CodeGen/CGValue.h:265:41 #12 0x00000000068d3fab getPointer /usr/local/google/home/kadircet/repos/llvm/clang/lib/CodeGen/CGValue.h:338:5 #13 0x00000000068d3fab clang::CodeGen::CodeGenFunction::EmitCallee(clang::Expr const*) /usr/local/google/home/kadircet/repos/llvm/clang/lib/CodeGen/CGExpr.cpp:5131:31 #14 0x00000000068d3c72 isBuiltin /usr/local/google/home/kadircet/repos/llvm/clang/lib/CodeGen/CGCall.h:151:12 #15 0x00000000068d3c72 clang::CodeGen::CodeGenFunction::EmitCallExpr(clang::CallExpr const*, clang::CodeGen::ReturnValueSlot) /usr/local/google/home/kadircet/repos/llvm/clang/lib/CodeGen/CGExpr.cpp:5014:14 #16 0x00000000069e0518 getInt /usr/local/google/home/kadircet/repos/llvm/llvm/include/llvm/ADT/PointerIntPair.h:62:57 #17 0x00000000069e0518 isScalar /usr/local/google/home/kadircet/repos/llvm/clang/lib/CodeGen/CGValue.h:54:37 #18 0x00000000069e0518 getScalarVal /usr/local/google/home/kadircet/repos/llvm/clang/lib/CodeGen/CGValue.h:62:5 #19 0x00000000069e0518 (anonymous namespace)::ScalarExprEmitter::VisitCallExpr(clang::CallExpr const*) /usr/local/google/home/kadircet/repos/llvm/clang/lib/CodeGen/CGExprScalar.cpp:578:36 #20 0x00000000069e1338 Visit /usr/local/google/home/kadircet/repos/llvm/clang/lib/CodeGen/CGExprScalar.cpp:408:52 #21 0x00000000069e1338 (anonymous namespace)::ScalarExprEmitter::VisitExprWithCleanups(clang::ExprWithCleanups*) /usr/local/google/home/kadircet/repos/llvm/clang/lib/CodeGen/CGExprScalar.cpp:2447:14 #22 0x00000000069d1128 Visit /usr/local/google/home/kadircet/repos/llvm/clang/lib/CodeGen/CGExprScalar.cpp:408:52 #23 0x00000000069d1128 clang::CodeGen::CodeGenFunction::EmitScalarExpr(clang::Expr const*, bool) /usr/local/google/home/kadircet/repos/llvm/clang/lib/CodeGen/CGExprScalar.cpp:4874:8 #24 0x00000000068b3a47 PointerIntPair /usr/local/google/home/kadircet/repos/llvm/llvm/include/llvm/ADT/PointerIntPair.h:49:12 #25 0x00000000068b3a47 RValue /usr/local/google/home/kadircet/repos/llvm/clang/lib/CodeGen/CGValue.h:39:7 #26 0x00000000068b3a47 get /usr/local/google/home/kadircet/repos/llvm/clang/lib/CodeGen/CGValue.h:90:12 #27 0x00000000068b3a47 clang::CodeGen::CodeGenFunction::EmitAnyExpr(clang::Expr const*, clang::CodeGen::AggValueSlot, bool) /usr/local/google/home/kadircet/repos/llvm/clang/lib/CodeGen/CGExpr.cpp:217:12 #28 0x00000000068b39d1 clang::CodeGen::CodeGenFunction::EmitIgnoredExpr(clang::Expr const*) /usr/local/google/home/kadircet/repos/llvm/clang/lib/CodeGen/CGExpr.cpp:206:1 #29 0x00000000068f2755 GetInsertBlock /usr/local/google/home/kadircet/repos/llvm/llvm/include/llvm/IR/IRBuilder.h:173:47 #30 0x00000000068f2755 clang::CodeGen::CodeGenFunction::EmitStmt(clang::Stmt const*, llvm::ArrayRef<clang::Attr const*>) /usr/local/google/home/kadircet/repos/llvm/clang/lib/CodeGen/CGStmt.cpp:123:42 #31 0x00000000068ff680 clang::CodeGen::CodeGenFunction::EmitCompoundStmtWithoutScope(clang::CompoundStmt const&, bool, clang::CodeGen::AggValueSlot) /usr/local/google/home/kadircet/repos/llvm/clang/lib/CodeGen/CGStmt.cpp:496:22 #32 0x00000000068e26fd clang::CodeGen::CodeGenFunction::EmitFunctionBody(clang::Stmt const*) /usr/local/google/home/kadircet/repos/llvm/clang/lib/CodeGen/CodeGenFunction.cpp:0:5 #33 0x00000000068e341e getLangOpts /usr/local/google/home/kadircet/repos/llvm/clang/lib/CodeGen/CodeGenFunction.h:1991:51 #34 0x00000000068e341e clang::CodeGen::CodeGenFunction::GenerateCode(clang::GlobalDecl, llvm::Function*, clang::CodeGen::CGFunctionInfo const&) /usr/local/google/home/kadircet/repos/llvm/clang/lib/CodeGen/CodeGenFunction.cpp:1443:7 #35 0x0000000006844ca5 clang::CodeGen::CodeGenModule::EmitGlobalFunctionDefinition(clang::GlobalDecl, llvm::GlobalValue*) /usr/local/google/home/kadircet/repos/llvm/clang/lib/CodeGen/CodeGenModule.cpp:5234:3 #36 0x000000000683d0cc clang::CodeGen::CodeGenModule::EmitGlobalDefinition(clang::GlobalDecl, llvm::GlobalValue*) /usr/local/google/home/kadircet/repos/llvm/clang/lib/CodeGen/CodeGenModule.cpp:0:9 #37 0x000000000682dea0 __normal_iterator /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_iterator.h:1027:20 #38 0x000000000682dea0 begin /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_vector.h:821:16 #39 0x000000000682dea0 empty /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_vector.h:1008:16 #40 0x000000000682dea0 clang::CodeGen::CodeGenModule::EmitDeferred() /usr/local/google/home/kadircet/repos/llvm/clang/lib/CodeGen/CodeGenModule.cpp:2675:26 #41 0x000000000682decc __normal_iterator /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_iterator.h:1027:20 #42 0x000000000682decc begin /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_vector.h:821:16 #43 0x000000000682decc empty /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_vector.h:1008:16 #44 0x000000000682decc clang::CodeGen::CodeGenModule::EmitDeferred() /usr/local/google/home/kadircet/repos/llvm/clang/lib/CodeGen/CodeGenModule.cpp:2677:7 #45 0x000000000682ba4a clang::CodeGen::CodeGenModule::Release() /usr/local/google/home/kadircet/repos/llvm/clang/lib/CodeGen/CodeGenModule.cpp:525:3 #46 0x0000000007169194 (anonymous namespace)::CodeGeneratorImpl::HandleTranslationUnit(clang::ASTContext&) /usr/local/google/home/kadircet/repos/llvm/clang/lib/CodeGen/ModuleBuilder.cpp:287:11 #47 0x000000000716660e clang::BackendConsumer::HandleTranslationUnit(clang::ASTContext&) /usr/local/google/home/kadircet/repos/llvm/clang/lib/CodeGen/CodeGenAction.cpp:306:13 #48 0x000000000842759a __normal_iterator /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_iterator.h:1027:20 #49 0x000000000842759a begin /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_vector.h:812:16 #50 0x000000000842759a finalize<std::vector<std::unique_ptr<clang::TemplateInstantiationCallback, std::default_delete<clang::TemplateInstantiationCallback> >, std::allocator<std::unique_ptr<clang::TemplateInstantiationCallback, std::default_delete<clang::TemplateInstantiationCallback> > > > > /usr/local/google/home/kadircet/repos/llvm/clang/include/clang/Sema/TemplateInstCallback.h:54:16 #51 0x000000000842759a clang::ParseAST(clang::Sema&, bool, bool) /usr/local/google/home/kadircet/repos/llvm/clang/lib/Parse/ParseAST.cpp:205:3 #52 0x000000000708bff0 clang::FrontendAction::Execute() /usr/local/google/home/kadircet/repos/llvm/clang/lib/Frontend/FrontendAction.cpp:1041:10 #53 0x0000000006ffd11f getPtr /usr/local/google/home/kadircet/repos/llvm/llvm/include/llvm/Support/Error.h:271:42 #54 0x0000000006ffd11f operator bool /usr/local/google/home/kadircet/repos/llvm/llvm/include/llvm/Support/Error.h:234:16 #55 0x0000000006ffd11f clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) /usr/local/google/home/kadircet/repos/llvm/clang/lib/Frontend/CompilerInstance.cpp:1035:23 #56 0x000000000715ff73 clang::ExecuteCompilerInvocation(clang::CompilerInstance*) /usr/local/google/home/kadircet/repos/llvm/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:266:25 #57 0x000000000409958a cc1_main(llvm::ArrayRef<char const*>, char const*, void*) /usr/local/google/home/kadircet/repos/llvm/clang/tools/driver/cc1_main.cpp:250:15 #58 0x0000000004096e30 ExecuteCC1Tool(llvm::SmallVectorImpl<char const*>&) /usr/local/google/home/kadircet/repos/llvm/clang/tools/driver/driver.cpp:317:12 #59 0x0000000006e75bc2 operator() /usr/local/google/home/kadircet/repos/llvm/clang/lib/Driver/Job.cpp:407:30 #60 0x0000000006e75bc2 void llvm::function_ref<void ()>::callback_fn<clang::driver::CC1Command::Execute(llvm::ArrayRef<llvm::Optional<llvm::StringRef>>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*, bool*) const::$_1>(long) /usr/local/google/home/kadircet/repos/llvm/llvm/include/llvm/ADT/STLFunctionalExtras.h:45:12 #61 0x000000000645f737 operator() /usr/local/google/home/kadircet/repos/llvm/llvm/include/llvm/ADT/STLFunctionalExtras.h:0:12 #62 0x000000000645f737 llvm::CrashRecoveryContext::RunSafely(llvm::function_ref<void ()>) /usr/local/google/home/kadircet/repos/llvm/llvm/lib/Support/CrashRecoveryContext.cpp:425:3 #63 0x0000000006e75600 clang::driver::CC1Command::Execute(llvm::ArrayRef<llvm::Optional<llvm::StringRef>>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*, bool*) const /usr/local/google/home/kadircet/repos/llvm/clang/lib/Driver/Job.cpp:407:7 #64 0x0000000006e37329 clang::driver::Compilation::ExecuteCommand(clang::driver::Command const&, clang::driver::Command const*&, bool) const /usr/local/google/home/kadircet/repos/llvm/clang/lib/Driver/Compilation.cpp:200:15 #65 0x0000000006e3760e clang::driver::Compilation::ExecuteJobs(clang::driver::JobList const&, llvm::SmallVectorImpl<std::pair<int, clang::driver::Command const*>>&, bool) const /usr/local/google/home/kadircet/repos/llvm/clang/lib/Driver/Compilation.cpp:254:13 #66 0x0000000006e543c0 empty /usr/local/google/home/kadircet/repos/llvm/llvm/include/llvm/ADT/SmallVector.h:73:47 #67 0x0000000006e543c0 clang::driver::Driver::ExecuteCompilation(clang::driver::Compilation&, llvm::SmallVectorImpl<std::pair<int, clang::driver::Command const*>>&) /usr/local/google/home/kadircet/repos/llvm/clang/lib/Driver/Driver.cpp:1738:23 #68 0x000000000409644f clang_main(int, char**) /usr/local/google/home/kadircet/repos/llvm/clang/tools/driver/driver.cpp:513:21 #69 0x00007f8c230177fd __libc_start_main ./csu/../csu/libc-start.c:332:16 #70 0x00000000040938ba _start (/usr/local/google/home/kadircet/repos/llvm/build/bin/clang+0x40938ba) Whereas the same code fragment at previous commit https://reviews.llvm.org/rG0538e5431afdb1fa05bdcedf70ee502ccfcd112a yields an diagnostics and crash free compile. A little bit adjusted reproducer to get rid of warnings: $ cat a.cc template <class a> struct b { template <class c> void ab(c); template <class c> void ad(c p1) { ab(d()(p1)); } a d(); }; template <typename e> struct f { using aa = e; template <class g> struct j { g ag; template <class... h> auto operator()(h... p1) -> decltype(ag(p1...)); }; using ai = b<j<aa>>; template <class c> void ad(c p1) { i.ad(p1); } ai i; }; template <typename c, typename> struct l { void k(const c &); struct n { int operator()(const c &) const; }; using m = f<n>; m aj; }; template <typename c, typename al> void l<c, al>::k(const c &p1) { aj.ad(p1); } template <typename c, typename al> int l<c, al>::n::operator()(const c &) const { return 0; } template class l<int, int>; Comment Actions Reverted in df48e3fbcc8be1f4c04bd97517d12e662f54de75, @tstellar it needs to be cherry-picked into release branch as well. Comment Actions Thanks for letting me know, I will make sure this gets backported: https://github.com/llvm/llvm-project/issues/56928 Comment Actions FWIW this also causes a static_assert failure while building ROOT: In file included from /home/jhahnfel/ROOT/src/tmva/tmva/src/DNN/Architectures/Cpu/CpuBuffer.cxx:17:
In file included from /opt/rh/gcc-toolset-11/root/usr/lib/gcc/x86_64-redhat-linux/11/../../../../include/c++/11/memory:77:
In file included from /opt/rh/gcc-toolset-11/root/usr/lib/gcc/x86_64-redhat-linux/11/../../../../include/c++/11/bits/shared_ptr.h:53:
/opt/rh/gcc-toolset-11/root/usr/lib/gcc/x86_64-redhat-linux/11/../../../../include/c++/11/bits/shared_ptr_base.h:1110:4: error: static assertion failed due to requirement '__is_invocable<TMVA::DNN::TCpuBuffer<double>::TDestructor &, double **&>::value': deleter expression d(p) is well-formed
static_assert(__is_invocable<_Deleter&, _Yp*&>::value,
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/opt/rh/gcc-toolset-11/root/usr/lib/gcc/x86_64-redhat-linux/11/../../../../include/c++/11/bits/shared_ptr.h:178:11: note: in instantiation of function template specialization 'std::__shared_ptr<double *, __gnu_cxx::_S_atomic>::__shared_ptr<double *, TMVA::DNN::TCpuBuffer<double>::TDestructor, void>' requested here
: __shared_ptr<_Tp>(__p, std::move(__d)) { }
^
/home/jhahnfel/ROOT/src/tmva/tmva/src/DNN/Architectures/Cpu/CpuBuffer.cxx:42:14: note: in instantiation of function template specialization 'std::shared_ptr<double *>::shared_ptr<double *, TMVA::DNN::TCpuBuffer<double>::TDestructor, void>' requested here
fBuffer = std::shared_ptr<AReal *>(pointer, fDestructor);
^After the revert, at least this issue is gone when building with current main. Comment Actions FWIW this also causes compilation error on correct code using Boost: https://github.com/llvm/llvm-project/issues/56878 Comment Actions The revert doesn't apply cleanly, can someone try to fix it up: https://github.com/llvm/llvm-project/issues/56928#issuecomment-1205483442 | ||||||||||||||||||||||||||||||||||||
You have a trailing ; after the definition of a::operator+ please remove it.