YexuanXiao wrote: > > > > Thank you for this! > > > > I'd like to better understand the need for the changes because I have a > > > > few concerns. One concern is about compile time performance. But also, > > > > this means downstream consumers of the AST are going to have to react > > > > because they used to be able to look for a `size_t` node directly and > > > > now they have to resolve a qualified type instead. This may be > > > > acceptable, but it seems disruptive too. > > > > Also, there should be more test coverage for the changes showing that > > > > we actually do get the types correct in all the various circumstances. > > > > > > > > > The current inlay hint of clangd is `auto a: unsigned long = > > > sizeof(int);`, which is misleading. At the same time, it eliminates > > > certain conversions that clang-tidy or other cleanup tools might warn > > > about. The C and C++ standards state that the result type of such > > > expressions is `size_t`/`ptrdiff_t`, so while this may disrupt some > > > downstream assumptions about prior implementations, it aligns more > > > closely with the standard. I believe this is worthwhile, maybe there's a > > > faster way to implement it. > > > > > > Yes, but this doesn't exactly accomplish that. In C, you'll still get the > > underlying integer type unless there happens to be a typedef we can find, > > right? So you can spot a difference between: > > ``` > > sizeof(int); // returns an unsigned integer type > > #include <stddef.h> > > sizeof(int); // now returns a typedef to an unsigned integer type > > ``` > > > > > > > > > > > > > > > > > > > > > > > > right? > > What it seems like you're really after is making `size_t` and `ptrdiff_t` > > actual identifiable types instead of magic based on the target. e.g., > > `ASTContext::getSizeType()` should return a `QualType` representing > > `size_t` which has the expected width and alignment and other type > > properties as what we currently get based on the target. Then > > `__SIZE_TYPE__` results in this otherwise unutterable type name, but it's > > distinguishable from the underlying integer type despite being compatible > > with it. > > CC @zygoloid @rjmccall @erichkeane for additional opinions on this. > > I think you're on to something here actually. We should do something like we > do with the `std` namespace: We create an 'implicit' version of it when we > need it (materializing it as the 'right' thing), and 'set' it correctly when > we 'find' it. Then, we can just 'get' it whenever we need it, like for these. > > It does NOT make sense to allow the fallback to have a different textual type > based on the existence of the typedef.
I don't have a problem to this, but this approach is more complex and requires more professional work. https://github.com/llvm/llvm-project/pull/136542 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
