wchilders added inline comments.
================
Comment at: clang/lib/Sema/SemaOverload.cpp:1761
+ llvm::SaveAndRestore<bool> DisableIITracking(
+ S.RebuildingImmediateInvocation, true);
+
----------------
Tyker wrote:
> I don't think RebuildingImmediateInvocation should be used here since we are
> not rebuilding immediate invocations
>
> setSuppressAllDiagnostics or TentativeAnalysisScope seems more adapted.
These don't actually work for this use case due to the queuing nature of the
diagnostic. Given the hacky nature of this assertion as it stands, I didn't
want to further pollute the code base.
I'm open to suggestions. If you're proposing making `MarkDeclRefReferenced` not
queue in response to diagnostic suppression, that doesn't seem totally
unreasonable; we'll just have to add the additional condition to check the
diagnostic state in `MarkDeclRefReferenced`.
================
Comment at: clang/test/SemaCXX/cxx2a-consteval.cpp:131
+ consteval A() = default;
+ consteval ~A() = default; // expected-error {{destructor cannot be declared
consteval}}
+};
----------------
Tyker wrote:
> could you check that this diagnostic and those like it doesn't appear when
> the struct A isn't instantiated.
I duplicated this namespace without any of the instantiation, and it does seem
to trigger the diagnostic "destructor cannot be declared consteval" -- all
other diagnostics are silent. Is that particularly undesirable?
Also, WRT testing. would that the best option here (having a duplicated
namespace, "dependent_addressing_uninstantiated" -- without the line triggering
instantiation)?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85933/new/
https://reviews.llvm.org/D85933
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
