Skip to content

[C++20][Modules] Quote header unit name in preprocessor output (-E) #112883

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Oct 24, 2024
Merged

[C++20][Modules] Quote header unit name in preprocessor output (-E) #112883

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions clang/lib/Frontend/PrintPreprocessedOutput.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -952,13 +952,15 @@ static void PrintPreprocessedTokens(Preprocessor &PP, Token &Tok,
continue;
} else if (Tok.is(tok::annot_header_unit)) {
// This is a header-name that has been (effectively) converted into a
// module-name.
// module-name, print them inside quote.
// FIXME: The module name could contain non-identifier module name
// components. We don't have a good way to round-trip those.
// components and OS specific file paths components. We don't have a good
// way to round-trip those.
Module *M = reinterpret_cast<Module *>(Tok.getAnnotationValue());
std::string Name = M->getFullModuleName();
Callbacks->OS->write(Name.data(), Name.size());
Callbacks->HandleNewlinesInToken(Name.data(), Name.size());
*Callbacks->OS << '"';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the header was imported by <>?

Copy link
Contributor Author

@dmpolukhin dmpolukhin Oct 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a good question but I don't see easy way to propagate isAngled from Preprocessor::HandleHeaderIncludeOrImport to tokens that will be printed here. This information cannot be putted to Module because it is not a characteristic of the module but information how it was spelled (even pointer to Module* is passed as void* token annotation value). Spelling file path in quotes at least won't break C++ parsing of the output as it happens now. FIXME here already covers that information about spelled header unit should be preserved. But if you think it is required fix now, I'll create a new structure that will contain Module*, isAngled and spelled header-unit name to resolve FIXME completely - it will require more changes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I don't want to block such issues.

Callbacks->OS->write_escaped(Name);
*Callbacks->OS << '"';
} else if (Tok.is(tok::annot_embed)) {
// Manually explode the binary data out to a stream of comma-delimited
// integer values. If the user passed -dE, that is handled by the
Expand Down
4 changes: 4 additions & 0 deletions clang/lib/Lex/TokenConcatenation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,10 @@ static char GetFirstChar(const Preprocessor &PP, const Token &Tok) {
bool TokenConcatenation::AvoidConcat(const Token &PrevPrevTok,
const Token &PrevTok,
const Token &Tok) const {
// No space is required between header unit name in quote and semi.
if (PrevTok.is(tok::annot_header_unit) && Tok.is(tok::semi))
return false;

// Conservatively assume that every annotation token that has a printable
// form requires whitespace.
if (PrevTok.isAnnotation())
Expand Down
20 changes: 20 additions & 0 deletions clang/test/Headers/header_unit_preprocessed_output.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// RUN: rm -fR %t
// RUN: split-file %s %t
// RUN: cd %t
// RUN: %clang_cc1 -verify -std=c++20 -emit-header-unit -xc++-user-header bz0.h
// RUN: %clang_cc1 -verify -std=c++20 -fmodule-file=bz0.pcm -xc++-user-header bz1.h -E -o bz1.output.h
// RUN: FileCheck %s < bz1.output.h
// RUN: %clang_cc1 -std=c++20 -fmodule-file=bz0.pcm -emit-header-unit -xc++-user-header bz1.output.h

//--- bz0.h
// expected-no-diagnostics
#pragma once

void foo();

//--- bz1.h
// expected-no-diagnostics
import "bz0.h";

// CHECK: # 1 ".{{/|\\\\?}}bz1.h"
// CHECK: import ".{{/|\\\\?}}bz0.h";
4 changes: 2 additions & 2 deletions clang/test/Modules/cxx20-include-translation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,6 @@ export void charlie() {
}

// CHECK: #pragma clang module import ".{{/|\\\\?}}h1.h"
// CHECK: import .{{/|\\\\?}}h2.h
// CHECK: import .{{/|\\\\?}}h3.h
// CHECK: import ".{{/|\\\\?}}h2.h"
// CHECK: import ".{{/|\\\\?}}h3.h"
// CHECK-NOT: #pragma clang module import ".{{/|\\\\?}}h4.h"
Loading