efriedma added inline comments.
================ Comment at: clang/lib/Sema/SemaDecl.cpp:14254 int SectionFlags = ASTContext::PSF_Read; - if (var->getType().isConstQualified()) { - if (HasConstInit) ---------------- dblaikie wrote: > rnk wrote: > > rsmith wrote: > > > efriedma wrote: > > > > rnk wrote: > > > > > I think this is not compatible with MSVC. MSVC uses simple logic, it > > > > > doesn't look for mutable: https://gcc.godbolt.org/z/sj6d4saxx > > > > > > > > > > The const mutable struct appears in the myrdata section in that > > > > > example. > > > > > > > > > > I think the solution is to separate the flag logic from the pragma > > > > > stack selection logic, which has to remain MSVC-compatible. > > > > MSVC apparently looks at whether the variable is marked "const", and > > > > nothing else; it doesn't look at mutable, it doesn't look at whether > > > > the variable has a constant initializer. So the current code isn't > > > > right either; if we're trying to implement MSVC-compatible logic, we > > > > shouldn't check HasConstInit. > > > > > > > > That said, I'm not sure how precisely/in what modes we want to > > > > precisely emulate MSVC. Probably anything we do here will be confusing. > > > We should at least issue a warning if the sensible logic and the > > > MSVC-compatible calculation differ. @rnk, do you know how important it is > > > to follow the MSVC semantics in this regard? > > I think it depends on whether you think that users are primarily using > > these pragmas to override the default rdata/bss/data sections without any > > care about precisely what goes where, or if they are using them to do > > something finer grained. > > > > If I had to guess, I'd say it's more likely the former, given that > > `__declspec(allocate)` and `#pragma(section)` exist to handle cases where > > users are putting specific globals into specific sections. > > > > Which, if we follow Richard's suggestion, would mean warning when we put a > > global marked `const` into a writable section when `ConstSegStack` is > > non-empty. That seems reasonable. `-Wmicrosoft-const-seg` for the new > > warning group? > Does the MSVC situation only apply to custom sections? (presumably when not > customizing the section, MSVC gets it right and can support a const global > with a runtime initializer, mutable member, or mutating dtor?) > > I think this code still needs to be modified, since this is the code that > drives the error about incompatible sections. So it'll need to behave > differently depending on the target platform? Yes, the MSVC situation is specifically if you specify `#pragma const_seg`; without the pragma, it does what you'd expect. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156726/new/ https://reviews.llvm.org/D156726 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits