AaronBallman 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:
> 
> Actually, it might be even worse; I _think_ it's valid for a user to define a 
> typedef for `size_t` themselves so long as C standard library headers are not 
> included, because it's not a reserved identifier in that case. I'm asking on 
> the WG14 reflectors because it matters for a test case like:
> 
> ```
> typedef float size_t;
> static_assert(_Generic(sizeof(int), size_t : 1, default : 0));
> ```
> 
> where it's unclear whether that static assertion should pass or fail.

Okay, the typedef is valid (not using a reserved identifier), and the assertion 
should fail because `sizeof` is defined as returning the size type defined in 
stddef.h explicitly, not just size_t generically.

> TO ADD: see uses of buildImplicitTypedef for times we do basically the same 
> thing.

I don't think we can do that, at least for C, because that means this code 
would then be accepted when it shouldn't be, right?
```
void foo() {
  (void)sizeof(0);
  size_t x = 12; // size_t was implicitly defined due to the use of sizeof above
}
```

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

Reply via email to