jyknight wrote:
I think the bug this change was attempting to fix is actually the same as
#38554 and related bugs -- which, it appears, was not really fixed.
The underlying problem here is that `#pragma once` should should work
identically to ifndef guards, as far as what macros/decls are made visible to
the includer. However, with the current modules implementation, it fails to do
so. This PR addresses only one _symptom_ of that issue, but does not address
the underlying problem.
Additionally, I think the semantics change implemented here is a bad idea. It
significantly complicates the semantics of `#import`, in a way that I think is
justifiable, especially given that there is a way to fix the identified issue
in a more principled way.
Yes, `#import` was a bad and confusing idea, and should have been dropped 20
years ago, but adding more special cases to it does not make it better. A
header file being marked "textual" in a module-map does _not_ mean a file
should be be included multiple times -- it simply means that you didn't want to
build it into a module. Linking these two concepts together ties together
concepts which have no
If this PR was the only way to fix the identified issue, maybe it could via an
assumption that the current buggy `#pragma once` behavior is expected behavior
which should be preserved.
As such, I would propose this PR ought to be reverted, and work to fix the
underlying issue be done instead.
Here's a test case which I think demonstrates the actual bug -- it fails both
before and after this PR. If you modify 'header.h' to use include guards, then
it passes. This difference in behavior should not exist.
```
// RUN: rm -rf %t
// RUN: split-file %s %t
// RUN: %clang_cc1 -std=c17 -fmodules-cache-path=%t/no-lsv -fmodules
-fimplicit-module-maps -I%t %t/pragma-once-modules.c -verify
// RUN: %clang_cc1 -std=c17 -fmodules-cache-path=%t/lsv -fmodules
-fimplicit-module-maps -fmodules-local-submodule-visibility -I%t
%t/pragma-once-modules.c -verify
//--- pragma-once-modules.c
// expected-no-diagnostics
#include "one.h"
#include "header.h"
my_int test(void) {
return MY_MACRO;
}
//--- module.modulemap
module Submodules {
module one {
header "one.h"
export *
}
module two {
header "two.h"
export *
}
}
//--- one.h
#ifndef one_h
#define one_h
#endif
//--- two.h
#ifndef two_h
#define two_h
#include "header.h"
#endif
//--- header.h
#pragma once
#define MY_MACRO 5
typedef int my_int;
```
It produces the following unexpected output.
```
error: 'expected-error' diagnostics seen but not expected:
[...]/pragma-once-modules.c Line 4: missing '#include "header.h"'; 'my_int'
must be declared before it is used
[...]/pragma-once-modules.c Line 5: use of undeclared identifier 'MY_MACRO'
error: 'expected-note' diagnostics seen but not expected:
[...]/header.h Line 3: declaration here is not visible
```
https://github.com/llvm/llvm-project/pull/83660
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
