aaron.ballman added inline comments.
================
Comment at: clang/lib/AST/ExprConstant.cpp:2357-2361
+ if (SubobjectDecl) {
+ Info.FFDiag(DiagLoc, diag::note_constexpr_uninitialized) <<
SubobjectDecl;
+ Info.Note(SubobjectDecl->getLocation(),
+ diag::note_constexpr_subobject_declared_here);
+ }
----------------
hazohelet wrote:
> aaron.ballman wrote:
> > Hmm, this breaks one of the contracts of our constexpr evaluation engine,
> > doesn't it? My understanding is that if constexpr evaluation fails, we will
> > have emitted a note diagnostic for why it failed. But if the caller doesn't
> > pass a nonnull `SubobjectDecl`, we'll return `false` but we won't issue a
> > diagnostic.
> >
> > I'm surprised no tests lost notes as a result of this change, that suggests
> > we're missing test coverage for the cases where nullptr is passed in
> > explicitly to this function.
> Yeah, I was looking into when `SubobjectDecl` can be null here. I `assert`ed
> the non-nullness of `SubobjectDecl` before and found that there exists two
> lines of code
> (https://github.com/llvm/llvm-project/blob/22a3f974d35da89247c0396594f2e4cd592742eb/clang/test/SemaCXX/attr-weak.cpp#L49
> and
> https://github.com/llvm/llvm-project/blob/abf4a8cb15d4faf04ee0da14e37d7349d3bde9a1/clang/test/CodeGenCXX/weak-external.cpp#L97)
> in the test codes that nulls here.
> It seems they are doing the same thing, doing comparison against a pointer to
> a `[[gnu::weak]]` member. A simple reproducing code is here:
> https://godbolt.org/z/qn997n85n
> As you can see from the compiler explorer, there's no note emitted here
> before the patch.
> I inserted some `printf` into the code before this patch and confirmed
> `Info.FFDiag(DiagLoc, diag::note_constexpr_uninitialized) << true << Type`
> was actually called when compiling the reproducing code and that somehow it
> is ignored. FWIW, `SubobjectLoc.isValid()` was `false` here.
> It seems they are doing the same thing, doing comparison against a pointer to
> a [[gnu::weak]] member. A simple reproducing code is here:
> https://godbolt.org/z/qn997n85n
> As you can see from the compiler explorer, there's no note emitted here
> before the patch.
I see a note generated there:
```
<source>:4:41: note: comparison against pointer to weak member
'Weak::weak_method' can only be performed at runtime
constexpr bool val = &Weak::weak_method != nullptr;
^
```
The situations I'm concerned about are the changes to ExprConstant.cpp:2270 or
line 2399 and so on.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146358/new/
https://reviews.llvm.org/D146358
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
