Quuxplusone added inline comments.
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:9176
// Don't check the implicit member of the anonymous union type.
- // This is technically non-conformant, but sanity demands it.
+ // This is technically non-conformant, but validation tests demand it.
return false;
----------------
ZarkoCA wrote:
> Quuxplusone wrote:
> > Quuxplusone wrote:
> > > Quuxplusone wrote:
> > > > This comment seems incorrectly translated.
> > > This comment //still// seems incorrectly translated.
> > > Things we do "for sanity's sake" aren't necessarily //required//,
> > > technically; but we're doing them anyway, for sanity.
> > "Don't check ... but check it anyway"?
> Right, that didn't make sense :). I noticed that there were warnings for this
> case in SemaDecl.cpp AFAIU so edited the comment to state that. Should be
> better now?
Well, this version of the comment now gives the //impression// that it must be
clear to //someone//. ;) I don't understand its implications, but I also don't
know the code.
Specifically, I don't know what "This" refers to (grammatically) — "Anonymous
union types aren't conforming, but we support them anyway, and this is the
right thing to do with them"? "Our behavior in this case isn't conforming, but
it wouldn't make sense to do the conforming thing [wat]"? or something else?
More fundamentally to //my// confusion (but not to that hypothetical other
someone), I don't know what "the implicit member of the anonymous union type"
actually means (in terms of the arcane details of C++), i.e., I personally
don't know when this codepath is hit or what its effect is when it does get hit.
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:12260
// even if hidden by ordinary names, *except* in a dependent context
- // where it's important for the sanity of two-phase lookup.
+ // where it's important for the validation of two-phase lookup.
if (!IsInstantiation)
----------------
Quuxplusone wrote:
> This comment seems incorrectly translated.
Perhaps just `// where they may be used by two-phase lookup.` (But this is now
just a nit.)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114025/new/
https://reviews.llvm.org/D114025
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
