dgoldman created this revision.
dgoldman added reviewers: sammccall, doug.gregor.
Herald added subscribers: usaxena95, kadircet, arphaman.
Herald added a project: All.
dgoldman requested review of this revision.
Herald added subscribers: cfe-commits, ilya-biryukov.
Herald added projects: clang, clang-tools-extra.
Previously, if a `#pragma clang assume_nonnull begin` was at the
end of a premable with a `#pragma clang assume_nonnull end` at the
end of the main file, clang would diagnose an unterminated begin in
the preamble and an unbalanced end in the main file.
With this change, those errors no longer occur and the case above is
now properly handled. I've added a corresponding test to clangd,
which makes use of preambles, in order to verify this works as
expected.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D122179
Files:
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
clang/include/clang/Lex/PreprocessorOptions.h
clang/include/clang/Serialization/ASTBitCodes.h
clang/lib/Lex/PPLexerChange.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
Index: clang/lib/Serialization/ASTWriter.cpp
===================================================================
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -872,6 +872,7 @@
RECORD(PP_CONDITIONAL_STACK);
RECORD(DECLS_TO_CHECK_FOR_DEFERRED_DIAGS);
RECORD(PP_INCLUDED_FILES);
+ RECORD(PP_ASSUME_NONNULL_LOC);
// SourceManager Block.
BLOCK(SOURCE_MANAGER_BLOCK);
@@ -2299,6 +2300,15 @@
Stream.EmitRecord(PP_COUNTER_VALUE, Record);
}
+ // If we have an unterminated pragma assume non null, remember it since it
+ // might be at the end of the preamble.
+ SourceLocation AssumeNonNullLoc = PP.getPragmaAssumeNonNullLoc();
+ if (AssumeNonNullLoc.isValid()) {
+ AddSourceLocation(AssumeNonNullLoc, Record);
+ Stream.EmitRecord(PP_ASSUME_NONNULL_LOC, Record);
+ Record.clear();
+ }
+
if (PP.isRecordingPreamble() && PP.hasRecordedPreamble()) {
assert(!IsModule);
auto SkipInfo = PP.getPreambleSkipInfo();
Index: clang/lib/Serialization/ASTReader.cpp
===================================================================
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -3109,6 +3109,7 @@
case IDENTIFIER_OFFSET:
case INTERESTING_IDENTIFIERS:
case STATISTICS:
+ case PP_ASSUME_NONNULL_LOC:
case PP_CONDITIONAL_STACK:
case PP_COUNTER_VALUE:
case SOURCE_LOCATION_OFFSETS:
@@ -3371,6 +3372,13 @@
}
break;
+ case PP_ASSUME_NONNULL_LOC: {
+ unsigned Idx = 0;
+ if (!Record.empty())
+ PP.setPragmaAssumeNonNullLoc(ReadSourceLocation(F, Record, Idx));
+ break;
+ }
+
case PP_CONDITIONAL_STACK:
if (!Record.empty()) {
unsigned Idx = 0, End = Record.size() - 1;
Index: clang/lib/Lex/PPLexerChange.cpp
===================================================================
--- clang/lib/Lex/PPLexerChange.cpp
+++ clang/lib/Lex/PPLexerChange.cpp
@@ -427,10 +427,13 @@
PragmaARCCFCodeAuditedInfo = {nullptr, SourceLocation()};
}
- // Complain about reaching a true EOF within assume_nonnull.
+ // Complain about reaching a true EOF within assume_nonnull unless we're
+ // building a preamble.
+ //
// We don't want to complain about reaching the end of a macro
// instantiation or a _Pragma.
- if (PragmaAssumeNonNullLoc.isValid() &&
+ if (PragmaAssumeNonNullLoc.isValid() && !this->PPOpts->GeneratePreamble &&
+ !(CurLexer && CurLexer->getFileID() == PredefinesFileID) &&
!isEndOfMacro && !(CurLexer && CurLexer->Is_PragmaLexer)) {
Diag(PragmaAssumeNonNullLoc, diag::err_pp_eof_in_assume_nonnull);
Index: clang/include/clang/Serialization/ASTBitCodes.h
===================================================================
--- clang/include/clang/Serialization/ASTBitCodes.h
+++ clang/include/clang/Serialization/ASTBitCodes.h
@@ -698,6 +698,9 @@
/// Record code for included files.
PP_INCLUDED_FILES = 66,
+
+ /// Record code for an unterminated \#pragma clang assume_nonnull begin.
+ PP_ASSUME_NONNULL_LOC = 67,
};
/// Record types used within a source manager block.
Index: clang/include/clang/Lex/PreprocessorOptions.h
===================================================================
--- clang/include/clang/Lex/PreprocessorOptions.h
+++ clang/include/clang/Lex/PreprocessorOptions.h
@@ -128,7 +128,8 @@
///
/// When the lexer is done, one of the things that need to be preserved is the
/// conditional #if stack, so the ASTWriter/ASTReader can save/restore it when
- /// processing the rest of the file.
+ /// processing the rest of the file. In addition, for preambles, we keep track
+ /// of an unterminated pragma assume nonnull.
bool GeneratePreamble = false;
/// Whether to write comment locations into the PCH when building it.
Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -745,6 +745,36 @@
EXPECT_THAT(TU.build().getLocalTopLevelDecls(), SizeIs(1));
}
+TEST(DiagnosticsTest, PreambleWithPragmaAssumeNonnull) {
+ Annotations Header(R"cpp(
+__attribute__((objc_root_class))
+@interface BaseClass
+- (id)description;
+- (void)dump:(id)str;
+@end
+)cpp");
+ auto TU = TestTU::withCode(R"cpp(
+#include "header.h"
+
+#if defined(__has_attribute) && __has_feature(nullability)
+#pragma clang assume_nonnull begin
+#endif
+
+void printSomething(BaseClass *obj);
+
+void printSomething(BaseClass *obj) {
+ [obj dump:obj.description];
+}
+
+#if defined(__has_attribute) && __has_feature(nullability)
+#pragma clang assume_nonnull end
+#endif
+ )cpp");
+ TU.Filename = "foo.m";
+ TU.AdditionalFiles["header.h"] = std::string(Header.code());
+ EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty());
+}
+
TEST(DiagnosticsTest, InsideMacros) {
Annotations Test(R"cpp(
#define TEN 10
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
