[CXX-95] Client facing headers should not include windows.h Created: 13/Mar/13  Updated: 07/Jan/15  Resolved: 07/Jan/15

Status: Closed
Project: C++ Driver
Component/s: Implementation
Affects Version/s: None
Fix Version/s: None

Type: Bug Priority: Minor - P4
Reporter: Andrew Morrow (Inactive) Assignee: Unassigned
Resolution: Won't Fix Votes: 0
Labels: cxxmove, legacy-cxx, windows
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Related

 Description   

If you use the C++ client driver headers, you automatically include windows.h, and get WIN32_LEAN_AND_MEAN set, via our windows_basic.h header. Our driver shouldn't drag all of windows.h into scope just to talk to the database. windows_basic.h should only be included in .cpp scope, not header scope.

See HELP-78 for examples of where this causes problems in practice.



 Comments   
Comment by Eric Milkie [ 11/Jun/14 ]

We no longer have client-facing headers in the server code, so we're good there.

Comment by Andrew Morrow (Inactive) [ 11/Jun/14 ]

It isn't really relevant in the server, since the header files aren't meaningfully consumed outside of the project.

Interestingly, the server recently de-inlined timer. And now that we have C++11 atomics, it may be that we have effectively accomplished this for many consumers.

Comment by Tyler Brock [ 11/Jun/14 ]

I think we can resolve this ticket as within the CXX scope as the only two files left are the following:

src/mongo/platform/atomic_intrinsics_win32.h
src/mongo/util/timer-win32-inl.h

However, this is still unresolved in the server, perhaps this should have been a copy rather than a move so it gets resolved in both places? Would be interested to see what acm thinks.

Comment by Andrew Morrow (Inactive) [ 07/Apr/14 ]

Kicking this out to 'unsure' because it isn't clear that this is achievable without significant restructuring.

Comment by Tad Marshall [ 14/Mar/13 ]

We could also try to minimize our usage of well known Windows types like DWORD (same as uint32_t), keep code out of headers, etc., but I think there are cases where it will be very clumsy to avoid it entirely.

Comment by Andy Schwerin [ 14/Mar/13 ]

tad, milkie, I think that maybe Windows typedefs and #defines should be explicitly excepted from IWYU, in favor of the "include basic.h first in your cpp file" rule. That would get us out of including basic.h/windows_basic.h in header files safely. We should discuss.

Comment by Eric Milkie [ 14/Mar/13 ]

Including windows_basic.h in headers should be allowed, but not for C++ driver headers. We should be very careful about where windows_basic.h is included. Once we have a better C++ driver interface (one that properly adorns its functions with calling-standard macros), it should make it a lot easier to identify which headers are client-facing.

Comment by Tad Marshall [ 14/Mar/13 ]

So one upshot of this is that header files that use Windows #defines or typedefs will delegate responsibility for their definition to the source file that includes them; correct? This will be an explicit exception to "include what you use".

Comment by Eric Milkie [ 14/Mar/13 ]

Should we change the Style Guide to mention that we should include basic.h as the first include for every cpp file, and to not include pch.h?

Comment by Andy Schwerin [ 13/Mar/13 ]

Header file fix-it day?

Comment by Andy Schwerin [ 13/Mar/13 ]

Problem headers are pch.h, basic.h and windows_basic.h. None of those should be included in other header files without prior authorization. All cpp files should have "mongo/platform/basic.h" as their very first include.

$ grep -Irl --include="*.h" -E '(pch|mongo/platform/(windows_)?basic)\.h"' src/mongo
src/mongo/bson/util/atomic_int.h
src/mongo/client/dbclient.h
src/mongo/client/dbclient_rs.h
src/mongo/client/dbclientcursor.h
src/mongo/client/dbclientinterface.h
src/mongo/client/distlock.h
src/mongo/db/btree.h
src/mongo/db/client.h
src/mongo/db/clientcursor.h
src/mongo/db/cursor.h
src/mongo/db/db.h
src/mongo/db/dbhelpers.h
src/mongo/db/extsort.h
src/mongo/db/geo/core.h
src/mongo/db/geo/hash.h
src/mongo/db/geo/shapes.h
src/mongo/db/hasher.h
src/mongo/db/index.h
src/mongo/db/indexkey.h
src/mongo/db/interrupt_status.h
src/mongo/db/interrupt_status_mongod.h
src/mongo/db/introspect.h
src/mongo/db/jsobj.h
src/mongo/db/namespace.h
src/mongo/db/namespace_details.h
src/mongo/db/ops/delete.h
src/mongo/db/ops/query.h
src/mongo/db/ops/update.h
src/mongo/db/ops/update_internal.h
src/mongo/db/pipeline/accumulator.h
src/mongo/db/pipeline/builder.h
src/mongo/db/pipeline/doc_mem_monitor.h
src/mongo/db/pipeline/document_source.h
src/mongo/db/pipeline/expression.h
src/mongo/db/pipeline/expression_context.h
src/mongo/db/pipeline/field_path.h
src/mongo/db/pipeline/pipeline.h
src/mongo/db/pipeline/pipeline_d.h
src/mongo/db/projection.h
src/mongo/db/repl_block.h
src/mongo/db/stats/counters.h
src/mongo/db/stats/snapshots.h
src/mongo/pch.h
src/mongo/platform/atomic_intrinsics_win32.h
src/mongo/s/balance.h
src/mongo/s/client_info.h
src/mongo/s/cursors.h
src/mongo/s/d_chunk_manager.h
src/mongo/s/d_logic.h
src/mongo/s/d_writeback.h
src/mongo/s/interrupt_status_mongos.h
src/mongo/s/request.h
src/mongo/s/shard.h
src/mongo/s/strategy.h
src/mongo/s/writeback_listener.h
src/mongo/tools/stat_util.h
src/mongo/util/bson_util.h
src/mongo/util/checksum.h
src/mongo/util/concurrency/mutex.h
src/mongo/util/concurrency/spin_lock.h
src/mongo/util/file.h
src/mongo/util/file_allocator.h
src/mongo/util/hashtab.h
src/mongo/util/lruishmap.h
src/mongo/util/net/httpclient.h
src/mongo/util/net/message_server.h
src/mongo/util/net/miniwebserver.h
src/mongo/util/net/sock.h
src/mongo/util/queue.h
src/mongo/util/stacktrace.h
src/mongo/util/string_writer.h
src/mongo/util/timer-win32-inl.h
src/mongo/util/trace.h
src/mongo/util/winutil.h

Generated at Wed Feb 07 21:58:11 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.