Skip to content

Commit

Permalink
(Issue #66) Map numerical constant and empty macros into the _ module…
Browse files Browse the repository at this point in the history 
…, if their headers aren't part of a Clang module.
  • Loading branch information
@Syniurge
Syniurge committed Jan 17, 2018
1 parent affee89 commit da8a975
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 14 deletions.
7 changes: 2 additions & 5 deletions ddmd/cpp/calypso.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -468,8 +468,8 @@ Loc fromLoc(clang::SourceLocation L)

auto S = SrcMgr.getFilename(L);
loc.filename = S.data();
assert(*(S.data() + S.size()) == '\0'); // TEMPORARY assert to confirm that StringRef isn't needed anymore
loc.linnum = ast()->getSourceManager().getSpellingLineNumber(L);
assert(!S.data() || *(S.data() + S.size()) == '\0'); // TEMPORARY assert to confirm that StringRef isn't needed anymore
loc.linnum = SrcMgr.getSpellingLineNumber(L);

return loc;
}
Expand Down Expand Up @@ -957,9 +957,6 @@ void LangPlugin::buildMacroMap()
if (FoundHeader) break;
}

if (!FoundHeader)
continue;

auto& MacroMapEntry = MacroMap[FoundHeader];
if (!MacroMapEntry)
MacroMapEntry = new MacroMapEntryTy;
Expand Down
24 changes: 15 additions & 9 deletions ddmd/cpp/cppmodule.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1502,6 +1502,17 @@ static inline bool isTopLevelInNamespaceModule (const clang::Decl *D)
return true;
}

void mapMacros(DeclMapper &mapper, clang::Module::Header* Header, Dsymbols *members)
{
auto MacroMapEntry = calypso.MacroMap[Header];
if (!MacroMapEntry)
return;

for (auto& P: *MacroMapEntry)
if (auto s = mapper.VisitMacro(P.first, P.second))
members->push(s);
}

static void mapNamespace(DeclMapper &mapper,
const clang::DeclContext *DC,
Dsymbols *members,
Expand Down Expand Up @@ -1540,6 +1551,9 @@ static void mapNamespace(DeclMapper &mapper,
if (auto s = mapper.VisitDecl(*D))
members->append(s);
}

if (DC->isTranslationUnit())
mapMacros(mapper, nullptr, members);
}

static void mapClangModule(DeclMapper &mapper,
Expand Down Expand Up @@ -1641,15 +1655,7 @@ static void mapClangModule(DeclMapper &mapper,
{
// Map the macros contained in the module headers (currently limited to numerical constants)
for (auto& Header: M->Headers[clang::Module::HK_Normal])
{
auto MacroMapEntry = calypso.MacroMap[&Header];
if (!MacroMapEntry)
continue;

for (auto& P: *MacroMapEntry)
if (auto s = mapper.VisitMacro(P.first, P.second))
members->push(s);
}
mapMacros(mapper, &Header, members);

for (auto D: RegionDecls)
if (isa<clang::TranslationUnitDecl>(D->getDeclContext()))
Expand Down
17 changes: 17 additions & 0 deletions tests/calypso/macro.d
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// RUN: %ldc -cpp-args -std=c++11 -cpp-cachedir=%t.cache -of %t %s
// RUN: %t > %t.out
// RUN: FileCheck %s < %t.out

modmap (C++) "macro.h";

import (C++) _;
import std.stdio : writeln;

void main()
{
writeln("my_macro = ", my_macro);

This comment has been minimized.

Sign in to view

Copy link
@timotheecour

timotheecour Jan 18, 2018

Collaborator

// make sure known at compile time
static assert(my_macro == 42);

This comment has been minimized.

Sign in to view

Copy link
@Syniurge

Syniurge Jan 18, 2018

Author Owner

Good idea, adding it.
// CHECK: my_macro = 42

writeln("empty_macro = ", empty_macro);
// CHECK: empty_macro = true
}
4 changes: 4 additions & 0 deletions tests/calypso/macro.h
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#pragma once

#define my_macro 42
#define empty_macro

3 comments on commit da8a975

@timotheecour
Copy link
Collaborator

Choose a reason for hiding this comment

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

thanks for the quick fix! (at least to non-function macros)
Instead of polluting global namespace, how about introducing it in another namespace, eg:

import (C++) _; // imports everything (but not macros)
import (C++) __macros; // imports all macros
import (C++) __macros : my_macro ; // ...

alternative name: __preprocessor

especially important because lots of #includes define tons of macros (often in a very non-trivial, system specific way that abuses the preprocessor)

@Syniurge
Copy link
Owner Author

Choose a reason for hiding this comment

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

I thought about that, but it would mean that if the author of a C++ library chooses to turn its #define constants into C++11 constexpr constants for example, user C++ code won't require any update, but D code would break (although a preemptive solution to support multiple versions of the library would be to import _). IMHO pollution is still preferable, kinda..

@timotheecour
Copy link
Collaborator

@timotheecour timotheecour commented on da8a975 Jan 18, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

  • I hear your point, here's a better option:
pragma(calypso_allow_macro)
import (C++) _  : cv_32F, cv_64F;

NOTE: using pragma is the way to go; it's flexible (eg we can attach a pragma to a single or multiple statements via pragma(x) foo;, pragma(x): foo; //till end, pragma(x){foo;...} ; they can be combined with other pragmas, and it allows for future features, eg:

pragma(calypso_recursive_namespace)
pragma(calypso_allow_macro)
import (C++) _  ; // imports all namespaces recursively

Please sign in to comment.