Skip to content

[Modules TS] #pragma once leaks out of module files in a way that macros do not #38554

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
llvmbot opened this issue Oct 7, 2018 · 9 comments
Closed
Labels
bugzilla Issues migrated from bugzilla clang:modules C++20 modules and Clang Header Modules confirmed Verified by a second party

Comments

@llvmbot
Copy link
Member

llvmbot commented Oct 7, 2018

Bugzilla Link 39206
Version 7.0
OS Windows NT
Attachments Repro with build script
Reporter LLVM Bugzilla Contributor
CC @compnerd,@davidstone,@dwblaikie,@DougGregor,@jyknight,@zygoloid,@yuanfang-chen

Extended Description

When I define a macro in a header that is included in both the implementation and interface modules the defined checks fail in the implementation module.

HEADER.H

#define MACRO

MODULE.CPPM

#include "HEADER.H"
export module MyModule;
#ifndef MACRO
#error MISSING MACRO
#end

IMPL.CPP

#include "HEADER.H"
module MyModule;
#ifndef MACRO
#error MISSING MACRO
#end

@llvmbot
Copy link
Member Author

llvmbot commented Oct 7, 2018

In the provided example the output is:

Expected:
No errors.

Actual:
The interface unit seems fine, but the implementation unit hits the error

@dwblaikie
Copy link
Collaborator

Looks like this is specifically related to the use of #pragma once. If you use classic header guards the file compiles as expected (the module definition doesn't expose the macros to the implementation file & the header inclusion in both places succeeds and provides the macro definitions locally).

I haven't read the modules-ts spec well enough to know if that's the intended behavior (I thought the idea was that modules could export macros in some way - but maybe they have to opt-in with some syntax, I'm not sure), but at least gets you on the way/provides a workaround or fix - and we can discuss here what the right behavior of #pragma once is. It's probably for it to behave like a header guard.

@llvmbot
Copy link
Member Author

llvmbot commented Nov 6, 2018

Yes, I was able to track this down to the header lookup deciding not to enter the header file a "second time" because the pcm has already seen the file and it had a pragma once. I was able to add a fix locally that does not use the header information coming from the pcm file (we can discuss if this is the correct fix if we decide this is the correct functionality).

From my reading of the TS it seems that the current iteration does not allow macros to cross module boundaries either through an import of as part of module linkage from interface unit to implementation unit. This may change with the adoption of legacy header modules coming in from the atom proposal (but I have not had time to dig into these changes).

As for the behavior of #pragma once, for now I would think each TU would process all header includes in isolation from each other, the ability to skip an include that was seen in a separate module file seems to be left over from the clang modules or from the use of the pch code paths.

@dwblaikie
Copy link
Collaborator

Ah, thanks - yeah, that all sounds spot-on to me.

@jyknight
Copy link
Member

See https://reviews.llvm.org/D26267 for a previous partial workaround for part of this issue, with some discussion.

I believe that set of hacks should be reverted when the issue is properly fixed.

@davidstone
Copy link
Contributor

Probably clear from the description, but this is not specific to macros in your header: the entire header is not included. If you try to reference a non-macro defined in the header, you end up with this comical error message:

c.cpp:6:2: error: missing '#include "a.hpp"'; 'f' must be declared before it is used
        f();
        ^
/home/david.stone/code/test/modules/a.hpp:3:6: note: previous declaration is here
void f() {
     ^
1 error generated.

given a source file like:

import b;

#include "a.hpp"

void g() {
	f();
}

Note that the file you defined as MODULE.CPPM must begin with the line module; for your code to be valid, but that does not affect this bug.

module;

#include "HEADER.H"

export module MyModule;

@davidstone
Copy link
Contributor

As another workaround, if you can build your header as a header unit, importing rather than including it into at least one of the module files avoids the problem.

@jyknight
Copy link
Member

Bug 39184 is probably either a duplicate or related.

@davidstone
Copy link
Contributor

Copying my minimal reproducer from #59708

// a.hpp
#pragma once
using a = int;
// b.hpp
#pragma once
#include "a.hpp"
a b;
// c.cpp
module;
#include "b.hpp"
export module c;

Causes Clang to error with

In file included from c.cpp:3:
b.hpp:4:1: error: unknown type name 'a'
a b;
^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla clang:modules C++20 modules and Clang Header Modules confirmed Verified by a second party
Projects
None yet
Development

No branches or pull requests

4 participants