erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land.
In D116792#3230478 <https://reviews.llvm.org/D116792#3230478>, @ChuanqiXu wrote: > In D116792#3227379 <https://reviews.llvm.org/D116792#3227379>, @erichkeane > wrote: > >> I had to do something similar for this at one point: >> https://github.com/llvm/llvm-project/commit/90010c2e1d60c6a9a4a0b30a113d4dae2b7214eb >> >> I seem to remember hitting this assert, and from my end, I think I decided >> even calling 'lookup' with the linkage spec to be a mistake (BTW, you might >> consider updating that 'Encloses' for 'export' as well!). > > Yeah, it is another bug for 'export'. I've tried to address it in > https://reviews.llvm.org/D116911 with the same style. > >> Is there any mechanism in the parent call of 'lookup' here to make it get >> the right thing? > > And 'lookup' is called in various places. For example, from the stack trace > of the crash, we could find that the parent of call is > `DeclareImplicitDeductionGuides`. And I tried to handle it in > `DeclareImplicitDeductionGuides`, then the compiler would crash again at > `LookupDirect` in `SemaLookup.cpp`. I feel it is not good to add checks for > places to call `lookup`. I agree that it is odd to lookup in a transparent > DeclContext. But I feel it is not bad to lookup in the enclosing context from > the definition of transparent DeclContext. Any thoughts? Hmm... I didn't realize this was the root 'lookup' function, and thank you for the above analysis. It seems to make more sense to me as well to have this be tolerant of this lookup setup. So LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116792/new/ https://reviews.llvm.org/D116792 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
