courbet added inline comments.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:13824
SourceLocation OpLoc) {
- if (Op->isTypeDependent())
+ if (Op->isTypeDependent() && !Op->getType()->isPointerType())
return S.Context.DependentTy;
----------------
aaron.ballman wrote:
> One thing that makes me a bit uncomfortable is that the logic for these used
> to be far more understandable when it was just checking for a dependent type.
> Now we need to sprinkle "and not a pointer type" in places, but it's not
> particularly clear as to why for a naïve reader of the code.
>
> I wonder if we want some sort of type predicate `isDeferrablyDependentType()`
> or something along those lines, or whether that's not really plausible?
> Basically, I'm hoping to find a way that, as a code reviewer, I can more
> easily spot places where `isTypeDependent()` should really be caring about
> type dependent pointers as a special case.
Right, so in this function `Op->isTypeDependent()` is always used to bail out
from type checking. The structure is typically:
```
if (E->isTypeDependent()) {
// bail out
}
// Actual type checking on provable types.
if (E->hasSomeTypeProperty1()) {
// OK case, do some property1-specific checking
} else if (E->hasSomeTypeProperty2()) {
// OK case, do some property2-specific checking
} else {
// Emit some error
}
```
My original approach was to do:
```
if (E->isTypeDependent() && !E->isPointerType()) {
// bail out
}
// Actual type checking on provable types or pointer types.
if (E->hasSomeTypeProperty1()) {
// OK case, do some property1-specific checking
} else if (E->hasSomeTypeProperty2()) {
// OK case, do some property2-specific checking
} else {
// Emit some error
}
```
It turns out that in all cases, `hasSomeTypeProperty1` is actually
`isPointerType` (or stuff like `isScalarType`, which includes pointers), so the
current code is actually already checking for pointerness, so in a sense my new
pointer type cheking is already included in subsequent checks, I'm not adding
anything new.
But maybe another change like this one will be able to prove more stuff about
dependent types (e.g. a dependent type could have propery2 too ), so what about
we only bail out on dependent types *after* we are done with the type checking:
```
if (E->isTypeDependent() && !E->isPointerType()) {
// bail out
}
// Actual type checking on provable types or pointer types.
if (E->hasSomeTypeProperty1()) {
// OK case, do some property1-specific checking
} else if (E->hasSomeTypeProperty2()) {
// OK case, do some property2-specific checking
} else if (if (E->isTypeDependent()) {
// bail out
} else {
// Emit some error
}
```
This essentially goes from "If the type is not dependent, try to prove stuff
about the type, else return error" to "try to prove stuff about the type, else
if not dependent, return error".
I'm modified the code here to use this approach, what do you think ?
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
