Page MenuHomePhabricator

[Modules] Emit warning if module cannot instantiate static class member.
AbandonedPublic

Authored by sepavloff on Aug 25 2015, 11:26 AM.

Details

Reviewers
None
Summary

Instantiation of static class members can be not obvious in some cases. Using
modules can cause problems even more difficult to diagnose. PR24425 describes
one of such cases. As a way to assist a user, compiler could issue warnings if
code that is potentially erroneous. This patch implements a warning if a static
class member is used in a module, but cannot be instantiated there.

Diff Detail

Event Timeline

sepavloff updated this revision to Diff 33094.Aug 25 2015, 11:26 AM
sepavloff retitled this revision from to [Modules] Emit warning if module cannot instantiate static class member..
sepavloff updated this object.
sepavloff added a subscriber: cfe-commits.

Any feedback?

Thanks,
--Serge

silvas added a subscriber: silvas.Sep 4 2015, 12:58 PM

How expensive is this check?

rsmith added a subscriber: rsmith.Sep 4 2015, 1:42 PM

I don't really see this as being specific to modules, nor to static data members. The natural generalization of this is:

  • at the end of a translation unit (including at the end of a module), warn on any implicit instantiations that are needed but could not be performed because the definition is not available

For valid code, this means we'd warn on templates/temploids which have their definition and all relevant explicit instantiations tucked away in some source file (but for which no explicit instantiation declarations are provided in the relevant header file) -- uncommon, but an idiom used by some. There is at least a syntactic solution we can suggest for people who see the warning: add the missing explicit instantiation declarations to the header file.

lib/Sema/SemaDecl.cpp
14444–14445

This is not the right way to implement the check. RecursiveASTVisitors are generally very expensive, and are poison to modules builds (because they will import all of the contents of all module files).

The right way to handle this is to produce the warning from within PerformPendingInstantiations when it's called at the end of the translation unit / module, in the cases where we want to perform an implicit instantiation, the template is not defined, and there is no preceding explicit instantiation declaration.

warning: instantiation of 'Foo<int>::s_bar' required here, but 'Foo::s_bar' is not defined
note: add an explicit instantiation declaration to suppress this warning if 'Foo<int>::s_bar' is explicitly instantiated in another translation unit

Extra credit: include a fixit hint in the note to add 'extern template char Foo<int>::s_bar;' somewhere sensible.

sepavloff abandoned this revision.Apr 20 2016, 1:09 AM
sepavloff removed subscribers: rsmith, silvas, sepavloff, cfe-commits.

Updated change D16396 implements this functionality.