Skip to content

[clang-cl] Fix value of __FUNCTION__ in MSVC mode. #84014

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 11 commits into from
Mar 19, 2024
Merged

[clang-cl] Fix value of __FUNCTION__ in MSVC mode. #84014

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
bdefe75
[clang-cl] Fix value of __FUNCTION__ in MSVC mode.
zahiraam Mar 5, 2024
13f4693
Addressed review comments.
zahiraam Mar 5, 2024
5eeffd1
Addressed review comments.
zahiraam Mar 6, 2024
16d3c75
Fixed typo.
zahiraam Mar 6, 2024
463cbc9
Addressed review comment.
zahiraam Mar 6, 2024
4b42f76
Refactor the code and added entry to TypePrinterTest.cpp.
zahiraam Mar 14, 2024
878b179
Fix format.
zahiraam Mar 14, 2024
bdcb510
Fix format again.
zahiraam Mar 14, 2024
efeb7b0
Responded review comments.
zahiraam Mar 18, 2024
9f16b25
Simplification applied.
zahiraam Mar 18, 2024
09b04af
Merge branch 'main' into ValueFuncMSVC
zahiraam Mar 19, 2024
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
3 changes: 3 additions & 0 deletions clang/docs/ReleaseNotes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,9 @@ Bug Fixes in This Version
for variables created through copy initialization having side-effects in C++17 and later.
Fixes (#GH64356) (#GH79518).

- Fix value of predefined macro ``__FUNCTION__`` in MSVC compatibility mode.
Fixes (#GH66114).

- Clang now emits errors for explicit specializations/instatiations of lambda call
operator.
Fixes (#GH83267).
Expand Down
3 changes: 2 additions & 1 deletion clang/include/clang/AST/Expr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2045,7 +2045,8 @@ class PredefinedExpr final
}

static std::string ComputeName(PredefinedIdentKind IK,
const Decl *CurrentDecl);
const Decl *CurrentDecl,
bool ForceElaboratedPrinting = false);

SourceLocation getBeginLoc() const { return getLocation(); }
SourceLocation getEndLoc() const { return getLocation(); }
Expand Down
22 changes: 21 additions & 1 deletion clang/lib/AST/DeclPrinter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -679,6 +679,16 @@ static void printExplicitSpecifier(ExplicitSpecifier ES, llvm::raw_ostream &Out,
Out << Proto;
}

static void MaybePrintTagKeywordIfSupressingScopes(PrintingPolicy &Policy,
QualType T,
llvm::raw_ostream &Out) {
StringRef prefix = T->isClassType() ? "class "
: T->isStructureType() ? "struct "
: T->isUnionType() ? "union "
: "";
Out << prefix;
}

void DeclPrinter::VisitFunctionDecl(FunctionDecl *D) {
if (!D->getDescribedFunctionTemplate() &&
!D->isFunctionTemplateSpecialization())
Expand Down Expand Up @@ -855,6 +865,10 @@ void DeclPrinter::VisitFunctionDecl(FunctionDecl *D) {
Out << Proto << " -> ";
Proto.clear();
}
if (!Policy.SuppressTagKeyword && Policy.SuppressScope &&
!Policy.SuppressUnwrittenScope)
MaybePrintTagKeywordIfSupressingScopes(Policy, AFT->getReturnType(),
Out);
AFT->getReturnType().print(Out, Policy, Proto);
Proto.clear();
}
Expand Down Expand Up @@ -1022,7 +1036,13 @@ void DeclPrinter::VisitVarDecl(VarDecl *D) {
? D->getIdentifier()->deuglifiedName()
: D->getName();

printDeclType(T, Name);
if (!Policy.SuppressTagKeyword && Policy.SuppressScope &&
!Policy.SuppressUnwrittenScope) {
MaybePrintTagKeywordIfSupressingScopes(Policy, T, Out);
printDeclType(T, Name);
} else {
printDeclType(T, Name);
}
Comment on lines +1039 to +1045
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if (!Policy.SuppressTagKeyword && Policy.SuppressScope &&
!Policy.SuppressUnwrittenScope) {
MaybePrintTagKeywordIfSupressingScopes(Policy, T, Out);
printDeclType(T, Name);
} else {
printDeclType(T, Name);
}
if (!Policy.SuppressTagKeyword && Policy.SuppressScope &&
!Policy.SuppressUnwrittenScope)
MaybePrintTagKeywordIfSupressingScopes(Policy, T, Out);
printDeclType(T, Name);


// Print the attributes that should be placed right before the end of the
// decl.
Expand Down
26 changes: 23 additions & 3 deletions clang/lib/AST/Expr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -676,7 +676,8 @@ StringRef PredefinedExpr::getIdentKindName(PredefinedIdentKind IK) {
// FIXME: Maybe this should use DeclPrinter with a special "print predefined
// expr" policy instead.
std::string PredefinedExpr::ComputeName(PredefinedIdentKind IK,
const Decl *CurrentDecl) {
const Decl *CurrentDecl,
bool ForceElaboratedPrinting) {
ASTContext &Context = CurrentDecl->getASTContext();

if (IK == PredefinedIdentKind::FuncDName) {
Expand Down Expand Up @@ -724,10 +725,21 @@ std::string PredefinedExpr::ComputeName(PredefinedIdentKind IK,
return std::string(Out.str());
}
if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(CurrentDecl)) {
if (IK != PredefinedIdentKind::PrettyFunction &&
const auto &LO = Context.getLangOpts();
bool IsFuncOrFunctionInNonMSVCCompatEnv =
((IK == PredefinedIdentKind::Func ||
IK == PredefinedIdentKind ::Function) &&
!LO.MSVCCompat);
bool IsLFunctionInMSVCCommpatEnv =
IK == PredefinedIdentKind::LFunction && LO.MSVCCompat;
bool IsFuncOrFunctionOrLFunctionOrFuncDName =
IK != PredefinedIdentKind::PrettyFunction &&
IK != PredefinedIdentKind::PrettyFunctionNoVirtual &&
IK != PredefinedIdentKind::FuncSig &&
IK != PredefinedIdentKind::LFuncSig)
IK != PredefinedIdentKind::LFuncSig;
if ((ForceElaboratedPrinting &&
(IsFuncOrFunctionInNonMSVCCompatEnv || IsLFunctionInMSVCCommpatEnv)) ||
(!ForceElaboratedPrinting && IsFuncOrFunctionOrLFunctionOrFuncDName))
return FD->getNameAsString();

SmallString<256> Name;
Expand Down Expand Up @@ -755,6 +767,8 @@ std::string PredefinedExpr::ComputeName(PredefinedIdentKind IK,
PrintingPolicy Policy(Context.getLangOpts());
PrettyCallbacks PrettyCB(Context.getLangOpts());
Policy.Callbacks = &PrettyCB;
if (IK == PredefinedIdentKind::Function && ForceElaboratedPrinting)
Policy.SuppressTagKeyword = !LO.MSVCCompat;
std::string Proto;
llvm::raw_string_ostream POut(Proto);

Expand Down Expand Up @@ -782,6 +796,12 @@ std::string PredefinedExpr::ComputeName(PredefinedIdentKind IK,

FD->printQualifiedName(POut, Policy);

if (IK == PredefinedIdentKind::Function) {
POut.flush();
Out << Proto;
return std::string(Name);
}

POut << "(";
if (FT) {
for (unsigned i = 0, e = Decl->getNumParams(); i != e; ++i) {
Expand Down
24 changes: 20 additions & 4 deletions clang/lib/AST/TypePrinter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1635,6 +1635,17 @@ void TypePrinter::printElaboratedBefore(const ElaboratedType *T,
if (T->getKeyword() != ElaboratedTypeKeyword::None)
OS << " ";
NestedNameSpecifier *Qualifier = T->getQualifier();
if (!Policy.SuppressTagKeyword && Policy.SuppressScope &&
!Policy.SuppressUnwrittenScope) {
bool OldTagKeyword = Policy.SuppressTagKeyword;
bool OldSupressScope = Policy.SuppressScope;
Policy.SuppressTagKeyword = true;
Policy.SuppressScope = false;
Comment on lines +1642 to +1643
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't we want to reset these once we're done with the call to printBefore()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I think so.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmmm, your changes don't look quite right to me. The return printBefore(...); is going to skip right over the restoration.

I think you wanted something more like:

      bool OldTag = Policy.SuppressTagKeyword, OldScope = Policy.SuppressScope;
      Policy.SuppressTagKeyword = true;
      Policy.SuppressScope = false;
      printBefore(T->getNamedType(), OS);
      Policy.SuppressTagKeyword = OldTag;
      Policy.SuppressScope = OldScope;
      return;

Before making the changes, I think you should also add test coverage to the unit test where these changes will fix the test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure what you mean here "where these changes will fix the test"? will fix which test? The unit test you are referring to is DeclPrinterTest? this should be TestCXXRecordDecl12.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The code was broken but no tests failed, then the fix you made was also broken but still no tests failed. So I'm asking you to write a new test that demonstrates incorrect behavior related to failing to reset the printing policy after modifying it, then fix the code until that test shows the correct behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm! Can't seem to find a test case that fails. At each step of the printing ElaboratedTypePolicyRAII PolicyRAII(Policy); gets called and that re-initializes the values of Policy.SuppressTagKeyword and Policy.SuppressScope to true!

Copy link
Collaborator

Choose a reason for hiding this comment

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

You need to reset the values here, or use a separate PrintingPolicy object so that you're not changing the value for the default printing policy used.

printBefore(T->getNamedType(), OS);
Policy.SuppressTagKeyword = OldTagKeyword;
Policy.SuppressScope = OldSupressScope;
return;
}
if (Qualifier)
Qualifier->print(OS, Policy);
}
Expand Down Expand Up @@ -2260,10 +2271,15 @@ printTo(raw_ostream &OS, ArrayRef<TA> Args, const PrintingPolicy &Policy,
} else {
if (!FirstArg)
OS << Comma;
// Tries to print the argument with location info if exists.
printArgument(Arg, Policy, ArgOS,
TemplateParameterList::shouldIncludeTypeForArgument(
Policy, TPL, ParmIndex));
if (!Policy.SuppressTagKeyword &&
Argument.getKind() == TemplateArgument::Type &&
isa<TagType>(Argument.getAsType()))
OS << Argument.getAsType().getAsString();
else
// Tries to print the argument with location info if exists.
printArgument(Arg, Policy, ArgOS,
TemplateParameterList::shouldIncludeTypeForArgument(
Policy, TPL, ParmIndex));
}
StringRef ArgString = ArgOS.str();

Expand Down
5 changes: 4 additions & 1 deletion clang/lib/Sema/SemaExpr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3740,7 +3740,10 @@ ExprResult Sema::BuildPredefinedExpr(SourceLocation Loc,
else {
// Pre-defined identifiers are of type char[x], where x is the length of
// the string.
auto Str = PredefinedExpr::ComputeName(IK, currentDecl);
bool ForceElaboratedPrinting =
IK == PredefinedIdentKind::Function && getLangOpts().MSVCCompat;
auto Str =
PredefinedExpr::ComputeName(IK, currentDecl, ForceElaboratedPrinting);
unsigned Length = Str.length();

llvm::APInt LengthI(32, Length + 1);
Expand Down
26 changes: 14 additions & 12 deletions clang/test/Analysis/eval-predefined-exprs.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,13 @@ void foo() {

struct A {
A() {
clang_analyzer_dump(__func__);
clang_analyzer_dump(__FUNCTION__);
clang_analyzer_dump(__PRETTY_FUNCTION__);
// expected-warning@-3 {{&Element{"A",0 S64b,char}}}
// expected-warning@-3 {{&Element{"A",0 S64b,char}}}
// expected-warning@-3 {{&Element{"A::A()",0 S64b,char}}}
clang_analyzer_dump(__func__); // expected-warning {{&Element{"A",0 S64b,char}}}
#ifdef ANALYZER_MS
clang_analyzer_dump(__FUNCTION__); // expected-warning {{&Element{"A::A",0 S64b,char}}}
#else
clang_analyzer_dump(__FUNCTION__); // expected-warning {{&Element{"A",0 S64b,char}}}
#endif
clang_analyzer_dump(__PRETTY_FUNCTION__); // expected-warning {{&Element{"A::A()",0 S64b,char}}}

#ifdef ANALYZER_MS
clang_analyzer_dump(__FUNCDNAME__);
Expand All @@ -71,12 +72,13 @@ struct A {
#endif
}
~A() {
clang_analyzer_dump(__func__);
clang_analyzer_dump(__FUNCTION__);
clang_analyzer_dump(__PRETTY_FUNCTION__);
// expected-warning@-3 {{&Element{"~A",0 S64b,char}}}
// expected-warning@-3 {{&Element{"~A",0 S64b,char}}}
// expected-warning@-3 {{&Element{"A::~A()",0 S64b,char}}}
clang_analyzer_dump(__func__); // expected-warning {{&Element{"~A",0 S64b,char}}}
#ifdef ANALYZER_MS
clang_analyzer_dump(__FUNCTION__); // expected-warning {{&Element{"A::~A",0 S64b,char}}}
#else
clang_analyzer_dump(__FUNCTION__); // expected-warning {{&Element{"~A",0 S64b,char}}}
#endif
clang_analyzer_dump(__PRETTY_FUNCTION__); // expected-warning {{&Element{"A::~A()",0 S64b,char}}}

#ifdef ANALYZER_MS
clang_analyzer_dump(__FUNCDNAME__);
Expand Down
70 changes: 66 additions & 4 deletions clang/test/SemaCXX/source_location.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions -verify %s
// RUN: %clang_cc1 -std=c++2a -fcxx-exceptions -DUSE_CONSTEVAL -fexceptions -verify %s
// RUN: %clang_cc1 -std=c++2b -fcxx-exceptions -DUSE_CONSTEVAL -DPAREN_INIT -fexceptions -verify %s
// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fms-extensions -DMS -fexceptions -verify %s
// RUN: %clang_cc1 -std=c++2a -fcxx-exceptions -fms-extensions -DMS -DUSE_CONSTEVAL -fexceptions -verify %s
// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fms-extensions -DMS -fexceptions -fms-compatibility -verify %s
// RUN: %clang_cc1 -std=c++2a -fcxx-exceptions -fms-extensions -DMS -DUSE_CONSTEVAL -fexceptions -fms-compatibility -verify %s
//
// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions -fexperimental-new-constant-interpreter -DNEW_INTERP -verify %s
// RUN: %clang_cc1 -std=c++2a -fcxx-exceptions -DUSE_CONSTEVAL -fexceptions -fexperimental-new-constant-interpreter -DNEW_INTERP -verify %s
// RUN: %clang_cc1 -std=c++2b -fcxx-exceptions -DUSE_CONSTEVAL -DPAREN_INIT -fexceptions -fexperimental-new-constant-interpreter -DNEW_INTERP -verify %s
// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fms-extensions -DMS -fexceptions -fexperimental-new-constant-interpreter -DNEW_INTERP -verify %s
// RUN: %clang_cc1 -std=c++2a -fcxx-exceptions -fms-extensions -DMS -DUSE_CONSTEVAL -fexceptions -fexperimental-new-constant-interpreter -DNEW_INTERP -verify %s
// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fms-extensions -DMS -fexceptions -fexperimental-new-constant-interpreter -DNEW_INTERP -fms-compatibility -verify %s
// RUN: %clang_cc1 -std=c++2a -fcxx-exceptions -fms-extensions -DMS -DUSE_CONSTEVAL -fexceptions -fexperimental-new-constant-interpreter -DNEW_INTERP -verify -fms-compatibility %s
// expected-no-diagnostics

#define assert(...) ((__VA_ARGS__) ? ((void)0) : throw 42)
Expand Down Expand Up @@ -463,8 +463,70 @@ void ctor_tests() {
constexpr SL global_sl = SL::current();
static_assert(is_equal(global_sl.function(), ""));

template <class T>
class TestBI {
public:
TestBI() {
#ifdef MS
static_assert(is_equal(__FUNCTION__, "test_func::TestBI<int>::TestBI"));
#else
static_assert(is_equal(__FUNCTION__, "TestBI"));
#endif
static_assert(is_equal(__func__, "TestBI"));
}
};

template <class T>
class TestClass {
public:
TestClass() {
#ifdef MS
static_assert(is_equal(__FUNCTION__, "test_func::TestClass<class test_func::C>::TestClass"));
#else
static_assert(is_equal(__FUNCTION__, "TestClass"));
#endif
static_assert(is_equal(__func__, "TestClass"));
}
};

template <class T>
class TestStruct {
public:
TestStruct() {
#ifdef MS
static_assert(is_equal(__FUNCTION__, "test_func::TestStruct<struct test_func::S>::TestStruct"));
#else
static_assert(is_equal(__FUNCTION__, "TestStruct"));
#endif
static_assert(is_equal(__func__, "TestStruct"));
}
};

template <class T>
class TestEnum {
public:
TestEnum() {
#ifdef MS
static_assert(is_equal(__FUNCTION__, "test_func::TestEnum<enum test_func::E>::TestEnum"));
#else
static_assert(is_equal(__FUNCTION__, "TestEnum"));
#endif
static_assert(is_equal(__func__, "TestEnum"));
}
};

class C {};
struct S {};
enum E {};

TestBI<int> t1;
TestClass<test_func::C> t2;
TestStruct<test_func::S> t3;
TestEnum<test_func::E> t4;

} // namespace test_func


//===----------------------------------------------------------------------===//
// __builtin_FUNCSIG()
//===----------------------------------------------------------------------===//
Expand Down
53 changes: 53 additions & 0 deletions clang/unittests/AST/DeclPrinterTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,59 @@ TEST(DeclPrinter, TestCXXRecordDecl11) {
"class A : virtual public Z, private Y {}"));
}

TEST(DeclPrinter, TestCXXRecordDecl12) {
ASSERT_TRUE(
PrintedDeclCXX98Matches("struct S { int x; };"
"namespace NS { class C {};}"
"void foo() {using namespace NS; C c;}",
"foo",
"void foo() {\nusing namespace NS;\nclass "
"NS::C c;\n}\n",
[](PrintingPolicy &Policy) {
Policy.SuppressTagKeyword = false;
Policy.SuppressScope = true;
Policy.TerseOutput = false;
}));
}

TEST(DeclPrinter, TestCXXRecordDecl13) {
ASSERT_TRUE(PrintedDeclCXX98Matches(
"struct S { int x; };"
"S s1;"
"S foo() {return s1;}",
"foo", "struct S foo() {\nreturn s1;\n}\n", [](PrintingPolicy &Policy) {
Policy.SuppressTagKeyword = false;
Policy.SuppressScope = true;
Policy.TerseOutput = false;
}));
}

TEST(DeclPrinter, TestCXXRecordDecl14) {
ASSERT_TRUE(PrintedDeclCXX98Matches(
"struct S { int x; };"
"S foo(S s1) {return s1;}",
"foo", "struct S foo(struct S s1) {\nreturn s1;\n}\n",
[](PrintingPolicy &Policy) {
Policy.SuppressTagKeyword = false;
Policy.SuppressScope = true;
Policy.TerseOutput = false;
}));
}
TEST(DeclPrinter, TestCXXRecordDecl15) {
ASSERT_TRUE(PrintedDeclCXX98Matches(
"struct S { int x; };"
"namespace NS { class C {};}"
"S foo(S s1, NS::C c1) {using namespace NS; C c; return s1;}",
"foo",
"struct S foo(struct S s1, class NS::C c1) {\nusing namespace NS;\nclass "
"NS::C c;\nreturn s1;\n}\n",
[](PrintingPolicy &Policy) {
Policy.SuppressTagKeyword = false;
Policy.SuppressScope = true;
Policy.TerseOutput = false;
}));
}

TEST(DeclPrinter, TestFunctionDecl1) {
ASSERT_TRUE(PrintedDeclCXX98Matches(
"void A();",
Expand Down
16 changes: 16 additions & 0 deletions clang/unittests/AST/TypePrinterTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,22 @@ TEST(TypePrinter, TemplateIdWithNTTP) {
}));
}

TEST(TypePrinter, TemplateArgumentsSubstitution) {
constexpr char Code[] = R"cpp(
template <typename Y> class X {};
typedef X<int> A;
int foo() {
return sizeof(A);
}
)cpp";
auto Matcher = typedefNameDecl(hasName("A"), hasType(qualType().bind("id")));
ASSERT_TRUE(PrintedTypeMatches(Code, {}, Matcher, "X<int>",
[](PrintingPolicy &Policy) {
Policy.SuppressTagKeyword = false;
Policy.SuppressScope = true;
}));
}

TEST(TypePrinter, TemplateArgumentsSubstitution_Expressions) {
/// Tests clang::isSubstitutedDefaultArgument on TemplateArguments
/// that are of kind TemplateArgument::Expression
Expand Down