[PATCH] D134458: [AST] Add C++11 attribute 'msvc::no_unique_address'

2023-11-04 Thread Richard Dzenis via Phabricator via cfe-commits
RIscRIpt abandoned this revision.
RIscRIpt added a comment.

Abandoning due to lack of time and expertise. I might come back and tackle it 
in several months.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134458

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


[PATCH] D134458: [AST] Add C++11 attribute 'msvc::no_unique_address'

2022-09-26 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:3505
+  let LangOpts = [MicrosoftExt];
+  let Spellings = [CXX11<"msvc", "no_unique_address", 201803>];
+  let Subjects = SubjectList<[NonBitField], ErrorDiag>;

Does the `201803` value match what MSVC returns from `__has_cpp_attribute`?



Comment at: clang/test/AST/msvc-attrs.cpp:4
+// RUN: FileCheck -check-prefix CHECK-DIAG-NO-MSX %s < %t.stderr.txt
+
+struct Empty {};

Thanks for the start to the tests! You should also add CodeGen tests as well as 
Sema tests. The CodeGen tests are to make sure we're codegenning the correct 
offsets to members, and the Sema tests are to ensure we're matching diagnostic 
behavior with MSVC rather than accepting code MSVC rejects.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134458

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


[PATCH] D134458: [AST] Add C++11 attribute 'msvc::no_unique_address'

2022-09-22 Thread Richard Dzenis via Phabricator via cfe-commits
RIscRIpt planned changes to this revision.
RIscRIpt added a comment.

TODO: add proper ABI tests of `[[msvc::no_unique_address]]`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134458

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


[PATCH] D134458: [AST] Add C++11 attribute 'msvc::no_unique_address'

2022-09-22 Thread Richard Dzenis via Phabricator via cfe-commits
RIscRIpt updated this revision to Diff 462297.
RIscRIpt retitled this revision from "[AST] Add msvc-specific C++11 attribute 
'msvc::no_unique_address'" to "[AST] Add C++11 attribute 
'msvc::no_unique_address'".
RIscRIpt added a comment.

Add naive implementation ABI of [[msvc::no_unique_address]]


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134458

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/Attr.td
  clang/lib/AST/Decl.cpp
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/CodeGen/CGExprConstant.cpp
  clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/AST/msvc-attrs.cpp

Index: clang/test/AST/msvc-attrs.cpp
===
--- /dev/null
+++ clang/test/AST/msvc-attrs.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fms-extensions -fc++-abi=microsoft -triple=x86_64-pc-windows-msvc -std=c++20 -ast-dump %s | FileCheck %s
+// RUN: not %clang_cc1 -Werror=ignored-attributes -ast-dump %s 2> %t.stderr.txt
+// RUN: FileCheck -check-prefix CHECK-DIAG-NO-MSX %s < %t.stderr.txt
+
+struct Empty {};
+
+struct StructWithNUAField {
+  // CHECK: FieldDecl 0x{{[0-9a-f]+}}  col:37 e 'Empty':'Empty'
+  // CHECK-NEXT: MSNoUniqueAddressAttr
+  [[msvc::no_unique_address]] Empty e;
+  int f1;
+};
+
+// CHECK-DIAG-NO-MSX: msvc-attrs.cpp:[[@LINE+1]]:1: error: static assertion failed due to requirement 'sizeof(StructWithNUAField) == sizeof(int)' 
+static_assert(sizeof(StructWithNUAField) == sizeof(int));
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -578,8 +578,7 @@
   // according to the Itanium ABI.  The exception applies only to records,
   // not arrays of records, so we must also check whether we stripped off an
   // array type above.
-  if (isa(RT->getDecl()) &&
-  (WasArray || !FD->hasAttr()))
+  if (isa(RT->getDecl()) && (WasArray || !FD->hasNoUniqueAddress()))
 return false;
 
   return isEmptyRecord(Context, FT, AllowArrays);
@@ -7523,8 +7522,7 @@
   // do count.  So do anonymous bitfields that aren't zero-sized.
 
   // Like isSingleElementStruct(), ignore C++20 empty data members.
-  if (FD->hasAttr() &&
-  isEmptyRecord(getContext(), FD->getType(), true))
+  if (FD->hasNoUniqueAddress() && isEmptyRecord(getContext(), FD->getType(), true))
 continue;
 
   // Unlike isSingleElementStruct(), arrays do not count.
Index: clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
===
--- clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -744,7 +744,7 @@
 Prior->Data = getByteArrayType(bitsToCharUnits(llvm::alignTo(
 cast(Prior->Data)->getIntegerBitWidth(), 8)));
   else {
-assert(Prior->FD->hasAttr() &&
+assert(Prior->FD->hasNoUniqueAddress() &&
"should not have reused this field's tail padding");
 Prior->Data = getByteArrayType(
 Context.getTypeInfoDataSizeInChars(Prior->FD->getType()).Width);
Index: clang/lib/CodeGen/CGExprConstant.cpp
===
--- clang/lib/CodeGen/CGExprConstant.cpp
+++ clang/lib/CodeGen/CGExprConstant.cpp
@@ -755,7 +755,7 @@
 return false;
   // After emitting a non-empty field with [[no_unique_address]], we may
   // need to overwrite its tail padding.
-  if (Field->hasAttr())
+  if (Field->hasNoUniqueAddress())
 AllowOverwrite = true;
 } else {
   // Otherwise we have a bitfield.
@@ -856,7 +856,7 @@
 return false;
   // After emitting a non-empty field with [[no_unique_address]], we may
   // need to overwrite its tail padding.
-  if (Field->hasAttr())
+  if (Field->hasNoUniqueAddress())
 AllowOverwrite = true;
 } else {
   // Otherwise we have a bitfield.
Index: clang/lib/CodeGen/CGExprAgg.cpp
===
--- clang/lib/CodeGen/CGExprAgg.cpp
+++ clang/lib/CodeGen/CGExprAgg.cpp
@@ -2015,7 +2015,7 @@
 
 AggValueSlot::Overlap_t
 CodeGenFunction::getOverlapForFieldInit(const FieldDecl *FD) {
-  if (!FD->hasAttr() || !FD->getType()->isRecordType())
+  if (!FD->hasNoUniqueAddress() || !FD->getType()->isRecordType())
 return AggValueSlot::DoesNotOverlap;
 
   // If the field lies entirely within the enclosing class's nvsize, its tail
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -474,7 +474,7 @@
 
   // We are able to place the member variable at this offset.
   // Make sure to update