This is an archive of the discontinued LLVM Phabricator instance.

clang/test/Modules/implicit-modules-use-lock.m
22	This is a `CHECK-NOT: <match>` line (with a custom `-check-prefix`). It confirms that `<match>` does not occur at least until the next positive check matches. The `FileCheck` invocation that uses `CHECK-NO-LOCKS` has no other check lines, so this really means: "`remark:` does not match on any line". Note I also needed to add `-allow-empty` since with no diagnostics at all, `FileCheck`'s stdin will be empty. FYI, if you want to read more: `-check-prefix` is documented at https://www.llvm.org/docs/CommandGuide/FileCheck.html#the-filecheck-check-prefix-option `CHECK-NOT` is documented at https://www.llvm.org/docs/CommandGuide/FileCheck.html#the-check-not-directive Often in clang tests it's easier to use `-cc1 -verify` for diagnostics instead of manual `FileCheck`s (`expected-no-diagnostics` comment being the equivalent of `-allow-empty`). In the modules testcases, there's often a complicated setup that we want to run a lot of `RUN:` lines against where each one expects different diagnostics. Maybe we should add prefix support to `-verify` (or maybe it's there and no one told me...).

LGTM

clang/test/Modules/implicit-modules-use-lock.m
22	Ah, that makes sense, thanks!

This revision is now accepted and ready to land.Jan 30 2021, 10:31 AM

This revision was landed with ongoing or failed builds.Aug 12 2021, 3:59 PM

Closed by commit rGb714f73defc8: Frontend: Add -f{,no-}implicit-modules-uses-lock and -Rmodule-lock (authored by dexonsmith). · Explain Why

This revision was automatically updated to reflect the committed changes.

dexonsmith added a commit: rGb714f73defc8: Frontend: Add -f{,no-}implicit-modules-uses-lock and -Rmodule-lock.

arichardson added a subscriber: arichardson.Aug 15 2021, 3:32 PM

arichardson added inline comments.

clang/test/Modules/implicit-modules-use-lock.m
22	This could use e.g. `-verify=lock` and `-verify=lock` to avoid the Filecheck usage.

dexonsmith mentioned this in rGd8a08fae0af9: Clean up test for -f{,no-}implicit-modules-uses-lock.Aug 16 2021, 4:23 PM

dexonsmith added inline comments.Aug 16 2021, 4:25 PM

clang/test/Modules/implicit-modules-use-lock.m
22	Thanks! Fixed in d8a08fae0af9aa09c108b3e7c60f192249dd2098.

Revision Contents

Path

Size

clang/

include/

clang/

Basic/

DiagnosticFrontendKinds.td

2 lines

DiagnosticGroups.td

1 line

Driver/

Options.td

6 lines

Frontend/

FrontendOptions.h

9 lines

lib/

Frontend/

CompilerInstance.cpp

26 lines

test/

Modules/

implicit-modules-use-lock.m

23 lines

Diff 366142

clang/include/clang/Basic/DiagnosticFrontendKinds.td

Show First 20 Lines • Show All 223 Lines • ▼ Show 20 Lines	def warn_module_config_macro_undef : Warning<
"to configure the module">,		"to configure the module">,
InGroup<ConfigMacros>;		InGroup<ConfigMacros>;
def note_module_def_undef_here : Note<		def note_module_def_undef_here : Note<
"macro was %select{defined\|#undef'd}0 here">;		"macro was %select{defined\|#undef'd}0 here">;
def remark_module_build : Remark<"building module '%0' as '%1'">,		def remark_module_build : Remark<"building module '%0' as '%1'">,
InGroup<ModuleBuild>;		InGroup<ModuleBuild>;
def remark_module_build_done : Remark<"finished building module '%0'">,		def remark_module_build_done : Remark<"finished building module '%0'">,
InGroup<ModuleBuild>;		InGroup<ModuleBuild>;
		def remark_module_lock : Remark<"locking '%0' to build module '%1'">,
		InGroup<ModuleLock>;
def err_modules_embed_file_not_found :		def err_modules_embed_file_not_found :
Error<"file '%0' specified by '-fmodules-embed-file=' not found">,		Error<"file '%0' specified by '-fmodules-embed-file=' not found">,
DefaultFatal;		DefaultFatal;
def err_module_header_file_not_found :		def err_module_header_file_not_found :
Error<"module header file '%0' not found">, DefaultFatal;		Error<"module header file '%0' not found">, DefaultFatal;

def err_test_module_file_extension_version : Error<		def err_test_module_file_extension_version : Error<
"test module file extension '%0' has different version (%1.%2) than expected "		"test module file extension '%0' has different version (%1.%2) than expected "
▲ Show 20 Lines • Show All 68 Lines • Show Last 20 Lines

clang/include/clang/Basic/DiagnosticGroups.td

	Show First 20 Lines • Show All 459 Lines • ▼ Show 20 Lines
	def CXX11LongLong : DiagGroup<"c++11-long-long">;			def CXX11LongLong : DiagGroup<"c++11-long-long">;
	def LongLong : DiagGroup<"long-long", [CXX11LongLong]>;			def LongLong : DiagGroup<"long-long", [CXX11LongLong]>;
	def ImplicitlyUnsignedLiteral : DiagGroup<"implicitly-unsigned-literal">;			def ImplicitlyUnsignedLiteral : DiagGroup<"implicitly-unsigned-literal">;
	def MethodSignatures : DiagGroup<"method-signatures">;			def MethodSignatures : DiagGroup<"method-signatures">;
	def MismatchedParameterTypes : DiagGroup<"mismatched-parameter-types">;			def MismatchedParameterTypes : DiagGroup<"mismatched-parameter-types">;
	def MismatchedReturnTypes : DiagGroup<"mismatched-return-types">;			def MismatchedReturnTypes : DiagGroup<"mismatched-return-types">;
	def MismatchedTags : DiagGroup<"mismatched-tags">;			def MismatchedTags : DiagGroup<"mismatched-tags">;
	def MissingFieldInitializers : DiagGroup<"missing-field-initializers">;			def MissingFieldInitializers : DiagGroup<"missing-field-initializers">;
				def ModuleLock : DiagGroup<"module-lock">;
	def ModuleBuild : DiagGroup<"module-build">;			def ModuleBuild : DiagGroup<"module-build">;
	def ModuleImport : DiagGroup<"module-import">;			def ModuleImport : DiagGroup<"module-import">;
	def ModuleConflict : DiagGroup<"module-conflict">;			def ModuleConflict : DiagGroup<"module-conflict">;
	def ModuleFileExtension : DiagGroup<"module-file-extension">;			def ModuleFileExtension : DiagGroup<"module-file-extension">;
	def RoundTripCC1Args : DiagGroup<"round-trip-cc1-args">;			def RoundTripCC1Args : DiagGroup<"round-trip-cc1-args">;
	def NewlineEOF : DiagGroup<"newline-eof">;			def NewlineEOF : DiagGroup<"newline-eof">;
	def Nullability : DiagGroup<"nullability">;			def Nullability : DiagGroup<"nullability">;
	def NullabilityDeclSpec : DiagGroup<"nullability-declspec">;			def NullabilityDeclSpec : DiagGroup<"nullability-declspec">;
	▲ Show 20 Lines • Show All 833 Lines • Show Last 20 Lines

clang/include/clang/Driver/Options.td

This file is larger than 256 KB, so syntax highlighting is disabled by default.

Show First 20 Lines • Show All 5,295 Lines • ▼ Show 20 Lines	def fmodules_embed_file_EQ : Joined<["-"], "fmodules-embed-file=">,
MetaVarName<"<file>">,		MetaVarName<"<file>">,
HelpText<"Embed the contents of the specified file into the module file "		HelpText<"Embed the contents of the specified file into the module file "
"being compiled.">,		"being compiled.">,
MarshallingInfoStringVector<FrontendOpts<"ModulesEmbedFiles">>;		MarshallingInfoStringVector<FrontendOpts<"ModulesEmbedFiles">>;
def fmodules_embed_all_files : Joined<["-"], "fmodules-embed-all-files">,		def fmodules_embed_all_files : Joined<["-"], "fmodules-embed-all-files">,
HelpText<"Embed the contents of all files read by this compilation into "		HelpText<"Embed the contents of all files read by this compilation into "
"the produced module file.">,		"the produced module file.">,
MarshallingInfoFlag<FrontendOpts<"ModulesEmbedAllFiles">>;		MarshallingInfoFlag<FrontendOpts<"ModulesEmbedAllFiles">>;
		defm fimplicit_modules_use_lock : BoolOption<"f", "implicit-modules-use-lock",
		FrontendOpts<"BuildingImplicitModuleUsesLock">, DefaultTrue,
		NegFlag<SetFalse>,
		PosFlag<SetTrue, [],
		"Use filesystem locks for implicit modules builds to avoid "
		"duplicating work in competing clang invocations.">>;
// FIXME: We only need this in C++ modules / Modules TS if we might textually		// FIXME: We only need this in C++ modules / Modules TS if we might textually
// enter a different module (eg, when building a header unit).		// enter a different module (eg, when building a header unit).
def fmodules_local_submodule_visibility :		def fmodules_local_submodule_visibility :
Flag<["-"], "fmodules-local-submodule-visibility">,		Flag<["-"], "fmodules-local-submodule-visibility">,
HelpText<"Enforce name visibility rules across submodules of the same "		HelpText<"Enforce name visibility rules across submodules of the same "
"top-level module.">,		"top-level module.">,
MarshallingInfoFlag<LangOpts<"ModulesLocalVisibility">>,		MarshallingInfoFlag<LangOpts<"ModulesLocalVisibility">>,
ImpliedByAnyOf<[fmodules_ts.KeyPath, fcxx_modules.KeyPath]>;		ImpliedByAnyOf<[fmodules_ts.KeyPath, fcxx_modules.KeyPath]>;
▲ Show 20 Lines • Show All 1,064 Lines • Show Last 20 Lines

clang/include/clang/Frontend/FrontendOptions.h

Show First 20 Lines • Show All 283 Lines • ▼ Show 20 Lines	public:
unsigned ASTDumpLookups : 1;		unsigned ASTDumpLookups : 1;

/// Whether we include declaration type dumps in AST dumps.		/// Whether we include declaration type dumps in AST dumps.
unsigned ASTDumpDeclTypes : 1;		unsigned ASTDumpDeclTypes : 1;

/// Whether we are performing an implicit module build.		/// Whether we are performing an implicit module build.
unsigned BuildingImplicitModule : 1;		unsigned BuildingImplicitModule : 1;

		/// Whether to use a filesystem lock when building implicit modules.
		unsigned BuildingImplicitModuleUsesLock : 1;

/// Whether we should embed all used files into the PCM file.		/// Whether we should embed all used files into the PCM file.
unsigned ModulesEmbedAllFiles : 1;		unsigned ModulesEmbedAllFiles : 1;

/// Whether timestamps should be written to the produced PCH file.		/// Whether timestamps should be written to the produced PCH file.
unsigned IncludeTimestamps : 1;		unsigned IncludeTimestamps : 1;

/// Should a temporary file be used during compilation.		/// Should a temporary file be used during compilation.
unsigned UseTemporary : 1;		unsigned UseTemporary : 1;
▲ Show 20 Lines • Show All 156 Lines • ▼ Show 20 Lines	public:
FrontendOptions()		FrontendOptions()
: DisableFree(false), RelocatablePCH(false), ShowHelp(false),		: DisableFree(false), RelocatablePCH(false), ShowHelp(false),
ShowStats(false), TimeTrace(false), ShowVersion(false),		ShowStats(false), TimeTrace(false), ShowVersion(false),
FixWhatYouCan(false), FixOnlyWarnings(false), FixAndRecompile(false),		FixWhatYouCan(false), FixOnlyWarnings(false), FixAndRecompile(false),
FixToTemporaries(false), ARCMTMigrateEmitARCErrors(false),		FixToTemporaries(false), ARCMTMigrateEmitARCErrors(false),
SkipFunctionBodies(false), UseGlobalModuleIndex(true),		SkipFunctionBodies(false), UseGlobalModuleIndex(true),
GenerateGlobalModuleIndex(true), ASTDumpDecls(false),		GenerateGlobalModuleIndex(true), ASTDumpDecls(false),
ASTDumpLookups(false), BuildingImplicitModule(false),		ASTDumpLookups(false), BuildingImplicitModule(false),
ModulesEmbedAllFiles(false), IncludeTimestamps(true),		BuildingImplicitModuleUsesLock(true), ModulesEmbedAllFiles(false),
UseTemporary(true), AllowPCMWithCompilerErrors(false),		IncludeTimestamps(true), UseTemporary(true),
TimeTraceGranularity(500) {}		AllowPCMWithCompilerErrors(false), TimeTraceGranularity(500) {}

/// getInputKindForExtension - Return the appropriate input kind for a file		/// getInputKindForExtension - Return the appropriate input kind for a file
/// extension. For example, "c" would return Language::C.		/// extension. For example, "c" would return Language::C.
///		///
/// \return The input kind for the extension, or Language::Unknown if the		/// \return The input kind for the extension, or Language::Unknown if the
/// extension is not recognized.		/// extension is not recognized.
static InputKind getInputKindForExtension(StringRef Extension);		static InputKind getInputKindForExtension(StringRef Extension);
};		};

} // namespace clang		} // namespace clang

#endif // LLVM_CLANG_FRONTEND_FRONTENDOPTIONS_H		#endif // LLVM_CLANG_FRONTEND_FRONTENDOPTIONS_H

clang/lib/Frontend/CompilerInstance.cpp

Show First 20 Lines • Show All 1,325 Lines • ▼ Show 20 Lines
/// multiple instances will compete to create the same module. On timeout,		/// multiple instances will compete to create the same module. On timeout,
/// deletes the lock file in order to avoid deadlock from crashing processes or		/// deletes the lock file in order to avoid deadlock from crashing processes or
/// bugs in the lock file manager.		/// bugs in the lock file manager.
static bool compileModuleAndReadASTBehindLock(		static bool compileModuleAndReadASTBehindLock(
CompilerInstance &ImportingInstance, SourceLocation ImportLoc,		CompilerInstance &ImportingInstance, SourceLocation ImportLoc,
SourceLocation ModuleNameLoc, Module *Module, StringRef ModuleFileName) {		SourceLocation ModuleNameLoc, Module *Module, StringRef ModuleFileName) {
DiagnosticsEngine &Diags = ImportingInstance.getDiagnostics();		DiagnosticsEngine &Diags = ImportingInstance.getDiagnostics();

		Diags.Report(ModuleNameLoc, diag::remark_module_lock)
		<< ModuleFileName << Module->Name;

// FIXME: have LockFileManager return an error_code so that we can		// FIXME: have LockFileManager return an error_code so that we can
// avoid the mkdir when the directory already exists.		// avoid the mkdir when the directory already exists.
StringRef Dir = llvm::sys::path::parent_path(ModuleFileName);		StringRef Dir = llvm::sys::path::parent_path(ModuleFileName);
llvm::sys::fs::create_directories(Dir);		llvm::sys::fs::create_directories(Dir);

while (1) {		while (1) {
llvm::LockFileManager Locked(ModuleFileName);		llvm::LockFileManager Locked(ModuleFileName);
switch (Locked) {		switch (Locked) {
▲ Show 20 Lines • Show All 42 Lines • ▼ Show 20 Lines	if (!OutOfDate)
return false;		return false;

// The module may be out of date in the presence of file system races,		// The module may be out of date in the presence of file system races,
// or if one of its imports depends on header search paths that are not		// or if one of its imports depends on header search paths that are not
// consistent with this ImportingInstance. Try again...		// consistent with this ImportingInstance. Try again...
}		}
}		}

		/// Compile a module in a separate compiler instance and read the AST,
		/// returning true if the module compiles without errors, potentially using a
		/// lock manager to avoid building the same module in multiple compiler
		/// instances.
		static bool compileModuleAndReadAST(CompilerInstance &ImportingInstance,
		SourceLocation ImportLoc,
		SourceLocation ModuleNameLoc,
		Module *Module, StringRef ModuleFileName) {
		return ImportingInstance.getInvocation()
		.getFrontendOpts()
		.BuildingImplicitModuleUsesLock
		? compileModuleAndReadASTBehindLock(ImportingInstance, ImportLoc,
		ModuleNameLoc, Module,
		ModuleFileName)
		: compileModuleAndReadASTImpl(ImportingInstance, ImportLoc,
		ModuleNameLoc, Module,
		ModuleFileName);
		}

/// Diagnose differences between the current definition of the given		/// Diagnose differences between the current definition of the given
/// configuration macro and the definition provided on the command line.		/// configuration macro and the definition provided on the command line.
static void checkConfigMacro(Preprocessor &PP, StringRef ConfigMacro,		static void checkConfigMacro(Preprocessor &PP, StringRef ConfigMacro,
Module *Mod, SourceLocation ImportLoc) {		Module *Mod, SourceLocation ImportLoc) {
IdentifierInfo *Id = PP.getIdentifierInfo(ConfigMacro);		IdentifierInfo *Id = PP.getIdentifierInfo(ConfigMacro);
SourceManager &SourceMgr = PP.getSourceManager();		SourceManager &SourceMgr = PP.getSourceManager();

// If this identifier has never had a macro definition, then it could		// If this identifier has never had a macro definition, then it could
▲ Show 20 Lines • Show All 461 Lines • ▼ Show 20 Lines	ModuleLoadResult CompilerInstance::findOrCompileModuleAndReadAST(
if (getPreprocessorOpts().FailedModules &&		if (getPreprocessorOpts().FailedModules &&
getPreprocessorOpts().FailedModules->hasAlreadyFailed(ModuleName)) {		getPreprocessorOpts().FailedModules->hasAlreadyFailed(ModuleName)) {
getDiagnostics().Report(ModuleNameLoc, diag::err_module_not_built)		getDiagnostics().Report(ModuleNameLoc, diag::err_module_not_built)
<< ModuleName << SourceRange(ImportLoc, ModuleNameLoc);		<< ModuleName << SourceRange(ImportLoc, ModuleNameLoc);
return nullptr;		return nullptr;
}		}

// Try to compile and then read the AST.		// Try to compile and then read the AST.
if (!compileModuleAndReadASTBehindLock(*this, ImportLoc, ModuleNameLoc, M,		if (!compileModuleAndReadAST(*this, ImportLoc, ModuleNameLoc, M,
ModuleFilename)) {		ModuleFilename)) {
assert(getDiagnostics().hasErrorOccurred() &&		assert(getDiagnostics().hasErrorOccurred() &&
"undiagnosed error in compileModuleAndReadAST");		"undiagnosed error in compileModuleAndReadAST");
if (getPreprocessorOpts().FailedModules)		if (getPreprocessorOpts().FailedModules)
getPreprocessorOpts().FailedModules->addFailed(ModuleName);		getPreprocessorOpts().FailedModules->addFailed(ModuleName);
return nullptr;		return nullptr;
}		}

// Okay, we've rebuilt and now loaded the module.		// Okay, we've rebuilt and now loaded the module.
▲ Show 20 Lines • Show All 359 Lines • Show Last 20 Lines

clang/test/Modules/implicit-modules-use-lock.m

This file was added.

				// Confirm -fimplicit-modules-use-lock and -fno-implicit-modules-use-lock control
				// whether building a module triggers -Rmodule-lock, indirectly checking whether
				// a lock manager is being used.
				//
				// RUN: rm -rf %t.cache
				// RUN: %clang_cc1 -fmodules -fimplicit-module-maps \
				// RUN: -fimplicit-modules-use-lock -Rmodule-lock \
				// RUN: -fmodules-cache-path=%t.cache -I%S/Inputs/system-out-of-date \
				// RUN: -fsyntax-only %s -Wnon-modular-include-in-framework-module \
				// RUN: -Werror=non-modular-include-in-framework-module 2>&1 \
				// RUN: \| FileCheck %s -check-prefix=CHECK-LOCKS
				//
				// RUN: rm -rf %t.cache
				// RUN: %clang_cc1 -fmodules -fimplicit-module-maps \
				// RUN: -fno-implicit-modules-use-lock -Rmodule-lock \
				// RUN: -fmodules-cache-path=%t.cache -I%S/Inputs/system-out-of-date \
				// RUN: -fsyntax-only %s -Wnon-modular-include-in-framework-module \
				// RUN: -Werror=non-modular-include-in-framework-module 2>&1 \
				// RUN: \| FileCheck %s -check-prefix=CHECK-NO-LOCKS -allow-empty

				// CHECK-NO-LOCKS-NOT: remark:
				// CHECK-LOCKS: remark: locking '{{.*}}.pcm' to build module 'X' [-Rmodule-lock]
				jansvoboda11Unsubmitted Not Done Reply Inline Actions Where is the empty remark coming from? Is it useful to us in any way? jansvoboda11: Where is the empty remark coming from? Is it useful to us in any way?
				dexonsmithAuthorUnsubmitted Done Reply Inline Actions This is a `CHECK-NOT: <match>` line (with a custom `-check-prefix`). It confirms that `<match>` does not occur at least until the next positive check matches. The `FileCheck` invocation that uses `CHECK-NO-LOCKS` has no other check lines, so this really means: "`remark:` does not match on any line". Note I also needed to add `-allow-empty` since with no diagnostics at all, `FileCheck`'s stdin will be empty. FYI, if you want to read more: `-check-prefix` is documented at https://www.llvm.org/docs/CommandGuide/FileCheck.html#the-filecheck-check-prefix-option `CHECK-NOT` is documented at https://www.llvm.org/docs/CommandGuide/FileCheck.html#the-check-not-directive Often in clang tests it's easier to use `-cc1 -verify` for diagnostics instead of manual `FileCheck`s (`expected-no-diagnostics` comment being the equivalent of `-allow-empty`). In the modules testcases, there's often a complicated setup that we want to run a lot of `RUN:` lines against where each one expects different diagnostics. Maybe we should add prefix support to `-verify` (or maybe it's there and no one told me...). dexonsmith: This is a `CHECK-NOT: <match>` line (with a custom `-check-prefix`). It confirms that `<match>`…
				jansvoboda11Unsubmitted Not Done Reply Inline Actions Ah, that makes sense, thanks! jansvoboda11: Ah, that makes sense, thanks!
				arichardsonUnsubmitted Not Done Reply Inline Actions This could use e.g. `-verify=lock` and `-verify=lock` to avoid the Filecheck usage. arichardson: This could use e.g. `-verify=lock` and `-verify=lock` to avoid the Filecheck usage.
				dexonsmithAuthorUnsubmitted Done Reply Inline Actions Thanks! Fixed in d8a08fae0af9aa09c108b3e7c60f192249dd2098. dexonsmith: Thanks! Fixed in d8a08fae0af9aa09c108b3e7c60f192249dd2098.
				@import X;

This is an archive of the discontinued LLVM Phabricator instance.

Frontend: Add -f{,no-}implicit-modules-uses-lock and -Rmodule-lockClosedPublic

Details

Diff Detail

Event Timeline