void added inline comments.
================ Comment at: clang/lib/Sema/SemaDecl.cpp:18891 + // incomplete definition. + if (const auto *RD = dyn_cast<RecordDecl>(D)) + return !RD->isCompleteDefinition(); ---------------- nickdesaulniers wrote: > nickdesaulniers wrote: > > void wrote: > > > nickdesaulniers wrote: > > > > what about `EnumDecl`s? I suspect the shared common base `TagDecl` > > > > might be better to use? > > > > > > > > If it is, can you add a test? I'm guessing > > > > > > > > ``` > > > > struct foo { > > > > enum havent_seen_yet; > > > > enum havent_seen_yet2; > > > > } > > > > ``` > > > > would be the test case. > > > Would testing for a `TagDecl` be better here? > > I think so; common shared base and same logic for both cases here. Unless > > there's something other than RecordDecl and EnumDecl where this shouldn't > > apply. > > Unless there's something other than RecordDecl and EnumDecl where this > > shouldn't apply. > > The `TagDecl` constructor is only called from the `RecordDecl` and `EnumDecl` > constructors. So I think it's simpler to just check `TagDecl` base type > rather than each of the two derived types. I like that better. Done. :) ================ Comment at: clang/test/CodeGen/init-randomized-struct-fwd-decl.c:2 +// RUN: %clang_cc1 -triple=x86_64-unknown-linux -emit-llvm -frandomize-layout-seed=1234567890abcdef < %s | FileCheck %s +// RUN: %clang_cc1 -triple=x86_64-unknown-linux -emit-llvm -frandomize-layout-seed=1234567890abcdef -DFORWARD_DECL < %s | FileCheck -check-prefix=FWD-DECL %s +// PR60349 ---------------- nickdesaulniers wrote: > Is the second check prefix necessary? The check lines look the same to me, > unless I'm missing something. > > Removing it should let us also drop the redundant FWD-DECL lines below. It's there to check that the randomization doesn't change if the forward decl is outside of the structure. However, Windows yet again uses a different algorithm than Linux and generates a different randomized ordering. It's annoying. I can remove this check. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143300/new/ https://reviews.llvm.org/D143300 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits