courbet added a comment. In D112453#3103515 <https://reviews.llvm.org/D112453#3103515>, @rsmith wrote:
> I think this change is being made in an imperfect place, though. Instead of > putting a special case in here, how would we feel about making > `Type::isOverloadableType()` smarter, to return `false` for dependent types > that can't possibly be class or enum types (eg, dependent pointer types, > array types, and maybe more exotic things like function types, atomic types, > and complex types)? This would then apply to all operators, not only unary > `*`. Eg, we know the type of `&p` where `p` is `T*`, and we know the type of > `p + n` where `p` is `T*` and `n` is integral. That change might have a lot > more fallout, but it'd certainly be interesting to see what it breaks. Thanks for the suggestion ! I've improved test coverage for unary operators in rG1427742750ed <https://reviews.llvm.org/rG1427742750ed1fcd2ead639c4ec5178fc34c9257>. There was a minor breakage with this change in `Sema::DefaultLvalueConversion` which did not handle dependant pointers. Binary operators look like they break more things. Working on it now. > Is this change observable in some way? We should include a test with this > change that demonstrates what effect it has. (The existing test in this patch > passes with or without the change.) Yes, I agree. The idea of this test was just to check that adding this did not have side effects on how clang was handling what the standard mandates, but you've covered this in the first part of your answer, thanks. I don't think it's observable directly (I'm observing the type using `getType()` directly in my case), so I'll add an AST dump test, thanks for the suggestion. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112453/new/ https://reviews.llvm.org/D112453 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
