aaron.ballman added a comment.

In D133574#3811496 <https://reviews.llvm.org/D133574#3811496>, @inclyc wrote:

> Emm, is it necessary to add a `LangOpts` check so that this change only 
> applies to c2x? If clang was invoked without `-std=c2x`, should we just 
> accept `offsetof` with definitions?

No, I think we should treat this as a defect report and have the behavior be 
the same in all versions of C.



================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:1650
+def err_type_defined_in_offsetof : Error<
+  "%0 cannot be defined in '__builtin_offsetof'">;
 
----------------
inclyc wrote:
> aaron.ballman wrote:
> > We might want this change, we might not -- can you test the diagnostic 
> > behavior when using `#include <stddef.h>`? Does it print 
> > `__builtin_offsetof` in the following example?
> > ```
> > #include <stddef.h>
> > 
> > int main() {
> >   return offsetof(struct S { int a; }, a);
> > }
> > ```
> > when executed with `clang -fsyntax-only -ffreestanding -std=c2x test.c`
> > 
> > If it prints the builtin name, I think we'll want to look at the builtin 
> > token to see if it was expanded from a macro named `offsetof` to improve 
> > the diagnostic quality.
> ```
> local/offsetofcc.c:4:26: error: 'struct S' cannot be defined in 
> '__builtin_offsetof'
>   return offsetof(struct S { int a; }, a);
>                          ^
> 1 error generated.
> ```
> > If it prints the builtin name, I think we'll want to look at the builtin 
> > token to see if it was expanded from a macro named offsetof to improve the 
> > diagnostic quality.
> 
> OK
We have similar code for this here: 

https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaChecking.cpp#L13438


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133574/new/

https://reviews.llvm.org/D133574

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to