aaron.ballman added inline comments.

================
Comment at: clang/include/clang/Parse/Parser.h:2311
     case DeclSpecContext::DSC_association:
+    case DeclSpecContext::DSC_offsetof:
       return true;
----------------
Is this correct? I don't think we can deduce the type from `offsetof` through 
CTAD, can we?

https://godbolt.org/z/Kab6ahYe7

(This might be a good test case to add assuming we don't already have that 
coverage.)


================
Comment at: clang/include/clang/Sema/DeclSpec.h:2069
     case DeclaratorContext::Association:
+    case DeclaratorContext::OffsetOf:
       return true;
----------------
I don't think we can omit the identifier in this context -- this function is 
used to see whether you can do something like:
```
template <int>
void func(int) {
  try {
  } catch (int) {
  }
}
```
and `offsetof` seems quite different from that.


================
Comment at: clang/test/Sema/offsetof.c:79
+    int a;
+    struct B // no-error, struct B is not defined within __builtin_offsetof 
directly
+    {
----------------
I think this is defensible. The wording in the standard is "If the specified 
type defines a new type or if the specified member is a bit-field, the behavior 
is undefined." and the specified type in this case is `struct A`; that `struct 
A` happens to also define `struct B` is immaterial.

However, the intent behind the change to the rule is to support older 
implementations of `offsetof` to protect them from having to deal with a case 
like: `offsetof(struct S { int a, b }, b);` where `offsetof` is a macro and 
thus the comma between `a` and `b` is treated as a separator. So there's a part 
of me that wonders if we want to also support diagnosing this case. But then 
we'd have to look at the declarator context more recursively to see whether any 
of the contexts on the stack are an `offsetof` context and that might be tricky.

Thoughts?


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