[clang] 81739c3 - [Modules] Fix an identifier hiding a function-like macro definition. (#135471)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 16 10:14:08 PDT 2025
Author: Volodymyr Sapsai
Date: 2025-04-16T10:14:05-07:00
New Revision: 81739c39db11b7f9a4f3528c1c66b552e57b47e4
URL: https://github.com/llvm/llvm-project/commit/81739c39db11b7f9a4f3528c1c66b552e57b47e4
DIFF: https://github.com/llvm/llvm-project/commit/81739c39db11b7f9a4f3528c1c66b552e57b47e4.diff
LOG: [Modules] Fix an identifier hiding a function-like macro definition. (#135471)
We emit a macro definition only in a module defining it. But it means
that if another module has an identifier with the same name as the
macro, the users of such module won't be able to use the macro anymore.
Fix by storing that an identifier has a macro definition that's not in a
current module (`MacroDirectivesOffset == 0`). This way
`IdentifierLookupVisitor` knows not to stop at the first module with an
identifier but to keep checking included modules for the actual macro
definition.
Fixes issue #32040.
rdar://30258278
Added:
clang/test/Modules/macro-identifier-hiding.c
Modified:
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTReaderInternals.h
clang/lib/Serialization/ASTWriter.cpp
Removed:
################################################################################
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 02c31dff620ec..58d3d33b31644 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -1131,7 +1131,7 @@ IdentifierInfo *ASTIdentifierLookupTrait::ReadData(const internal_key_type& k,
bool HasRevertedTokenIDToIdentifier = readBit(Bits);
bool Poisoned = readBit(Bits);
bool ExtensionToken = readBit(Bits);
- bool HadMacroDefinition = readBit(Bits);
+ bool HasMacroDefinition = readBit(Bits);
assert(Bits == 0 && "Extra bits in the identifier?");
DataLen -= sizeof(uint16_t) * 2;
@@ -1151,14 +1151,17 @@ IdentifierInfo *ASTIdentifierLookupTrait::ReadData(const internal_key_type& k,
"Incorrect C++ operator keyword flag");
(void)CPlusPlusOperatorKeyword;
- // If this identifier is a macro, deserialize the macro
- // definition.
- if (HadMacroDefinition) {
+ // If this identifier has a macro definition, deserialize it or notify the
+ // visitor the actual definition is in a
diff erent module.
+ if (HasMacroDefinition) {
uint32_t MacroDirectivesOffset =
endian::readNext<uint32_t, llvm::endianness::little>(d);
DataLen -= 4;
- Reader.addPendingMacro(II, &F, MacroDirectivesOffset);
+ if (MacroDirectivesOffset)
+ Reader.addPendingMacro(II, &F, MacroDirectivesOffset);
+ else
+ hasMacroDefinitionInDependencies = true;
}
Reader.SetIdentifierInfo(ID, II);
@@ -2419,6 +2422,10 @@ namespace {
// declarations it needs.
++NumIdentifierLookupHits;
Found = *Pos;
+ if (Trait.hasMoreInformationInDependencies()) {
+ // Look for the identifier in extra modules as they contain more info.
+ return false;
+ }
return true;
}
diff --git a/clang/lib/Serialization/ASTReaderInternals.h b/clang/lib/Serialization/ASTReaderInternals.h
index 353e0a53cad9b..4a7794889b039 100644
--- a/clang/lib/Serialization/ASTReaderInternals.h
+++ b/clang/lib/Serialization/ASTReaderInternals.h
@@ -286,6 +286,8 @@ class ASTIdentifierLookupTrait : public ASTIdentifierLookupTraitBase {
// identifier that was constructed before the AST file was read.
IdentifierInfo *KnownII;
+ bool hasMacroDefinitionInDependencies = false;
+
public:
using data_type = IdentifierInfo *;
@@ -300,6 +302,10 @@ class ASTIdentifierLookupTrait : public ASTIdentifierLookupTraitBase {
IdentifierID ReadIdentifierID(const unsigned char *d);
ASTReader &getReader() const { return Reader; }
+
+ bool hasMoreInformationInDependencies() const {
+ return hasMacroDefinitionInDependencies;
+ }
};
/// The on-disk hash table used to contain information about
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 95b5718f1d140..8c261e13d5ea4 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -3795,7 +3795,10 @@ bool IsInterestingIdentifier(const IdentifierInfo *II, uint64_t MacroOffset,
II->getNotableIdentifierID() != tok::NotableIdentifierKind::not_notable ||
II->getBuiltinID() != Builtin::ID::NotBuiltin ||
II->getObjCKeywordID() != tok::ObjCKeywordKind::objc_not_keyword;
- if (MacroOffset || II->isPoisoned() || (!IsModule && IsInteresting) ||
+ if (MacroOffset ||
+ (II->hasMacroDefinition() &&
+ II->hasFETokenInfoChangedSinceDeserialization()) ||
+ II->isPoisoned() || (!IsModule && IsInteresting) ||
II->hasRevertedTokenIDToIdentifier() ||
(NeedDecls && II->getFETokenInfo()))
return true;
@@ -3874,7 +3877,8 @@ class ASTIdentifierTableTrait {
if (isInterestingIdentifier(II, MacroOffset)) {
DataLen += 2; // 2 bytes for builtin ID
DataLen += 2; // 2 bytes for flags
- if (MacroOffset)
+ if (MacroOffset || (II->hasMacroDefinition() &&
+ II->hasFETokenInfoChangedSinceDeserialization()))
DataLen += 4; // MacroDirectives offset.
if (NeedDecls && IdResolver)
@@ -3905,15 +3909,17 @@ class ASTIdentifierTableTrait {
assert((Bits & 0xffff) == Bits && "ObjCOrBuiltinID too big for ASTReader.");
LE.write<uint16_t>(Bits);
Bits = 0;
- bool HadMacroDefinition = MacroOffset != 0;
- Bits = (Bits << 1) | unsigned(HadMacroDefinition);
+ bool HasMacroDefinition =
+ (MacroOffset != 0) || (II->hasMacroDefinition() &&
+ II->hasFETokenInfoChangedSinceDeserialization());
+ Bits = (Bits << 1) | unsigned(HasMacroDefinition);
Bits = (Bits << 1) | unsigned(II->isExtensionToken());
Bits = (Bits << 1) | unsigned(II->isPoisoned());
Bits = (Bits << 1) | unsigned(II->hasRevertedTokenIDToIdentifier());
Bits = (Bits << 1) | unsigned(II->isCPlusPlusOperatorKeyword());
LE.write<uint16_t>(Bits);
- if (HadMacroDefinition)
+ if (HasMacroDefinition)
LE.write<uint32_t>(MacroOffset);
if (NeedDecls && IdResolver) {
diff --git a/clang/test/Modules/macro-identifier-hiding.c b/clang/test/Modules/macro-identifier-hiding.c
new file mode 100644
index 0000000000000..4cd7cf0500322
--- /dev/null
+++ b/clang/test/Modules/macro-identifier-hiding.c
@@ -0,0 +1,29 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache \
+// RUN: -fsyntax-only %t/test.c -verify
+// Test again with the populated module cache.
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache \
+// RUN: -fsyntax-only %t/test.c -verify
+
+// Test that an identifier with the same name as a macro doesn't hide this
+// macro from the includers.
+
+//--- macro-definition.h
+#define __P(protos) ()
+#define __Q(protos) ()
+
+//--- macro-transitive.h
+#include "macro-definition.h"
+void test(int __P) {} // not "interesting" identifier
+struct __Q {}; // "interesting" identifier
+
+//--- module.modulemap
+module MacroDefinition { header "macro-definition.h" export * }
+module MacroTransitive { header "macro-transitive.h" export * }
+
+//--- test.c
+// expected-no-diagnostics
+#include "macro-transitive.h"
+void foo __P(());
+void bar __Q(());
More information about the cfe-commits
mailing list