Skip to content
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.

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

CxxPreprocessor: cache forced includes, forced & global defines #1665

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
217 changes: 157 additions & 60 deletions cxx-squid/src/main/java/org/sonar/cxx/preprocessor/CxxPreprocessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@

public class CxxPreprocessor extends Preprocessor {

private static final String CPLUSPLUS = "__cplusplus";
private static final String EVALUATED_TO_FALSE = "[{}:{}]: '{}' evaluated to false, skipping tokens that follow";
private static final String MISSING_INCLUDE_MSG
= "Preprocessor: {} include directive error(s). This is only relevant if parser creates syntax errors."
Expand All @@ -90,8 +89,55 @@ public class CxxPreprocessor extends Preprocessor {
private File currentContextFile;

private final Parser<Grammar> pplineParser;
/**
* Contains a standard set of pre-compiled macros, which are defined for each
* compilation unit. This map is used if there is no specific compilation unit
* settings. The map contains
* <ol>
* <li>{@link Macro#STANDARD_MACROS}</li>
* <li>{@link CxxConfiguration#getDefines()} and</li>
* <li>forced includes (see
* {@link CxxConfiguration#getForceIncludeFiles()})</li>
* </ol>
* All hi-prio macros are pre-parsed while construction of
* {@link CxxPreprocessor}
*/
private final MapChain<String, Macro> fixedMacros = new MapChain<>();
/**
* If CxxConfiguration contains any compilation unit settings, this map is
* filled with a set of pre-compiled macros. Those macros must be defined for
* each compilation unit:
* <ol>
* <li>{@link Macro#UNIT_MACROS}</li>
* <li>{@link CxxConfiguration#getDefines()} and</li>
* <li>forced includes (see
* {@link CxxConfiguration#getForceIncludeFiles()})</li>
* </ol>
* The map is immutable; macros are pre-parsed while construction of
* {@link CxxPreprocessor}
*/
private final Map<String, Macro> predefinedUnitMacros;
/**
* If current processed file has some specific configuration settings, this
* map will be filled with relevant macros and defines:
* <ol>
* <li>predefined compilation unit macros will be added (see
* {@link CxxPreprocessor#predefinedUnitMacros}</li>
* <li>specific unit settings will be parsed and added (see
* {@link CxxConfiguration#getCompilationUnitSettings(String)}</li>
* </ol>
* Map is recalculated each time {@link CxxPreprocessor} is about to analyze a
* new file (see {@link CxxPreprocessor#init()}.
*
* If current processed file has no specific configuration settings -
* {@link CxxPreprocessor#fixedMacros} will be used.
*/
private MapChain<String, Macro> unitMacros;
/**
* Pre-parsed defines from the global compilation unit settings, see
* {@link CxxConfiguration#getGlobalCompilationUnitSettings()}.
*/
private final Map<String, Macro> globalCUDefines;
private final Set<File> analysedFiles = new HashSet<>();
private final SourceCodeProvider codeProvider;
private SourceCodeProvider unitCodeProvider;
Expand Down Expand Up @@ -127,34 +173,56 @@ public CxxPreprocessor(SquidAstVisitorContext<Grammar> context,

pplineParser = CppParser.create(conf);

final List<String> configuredDefines = conf.getDefines();
Copy link
Collaborator

Choose a reason for hiding this comment

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

@ivangalkin ctor code is difficult to read. Maybe you can split it into smaller methods.

Copy link
Contributor Author

@ivangalkin ivangalkin Jan 15, 2019

Choose a reason for hiding this comment

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

yes, I'll do some refactoring;

Map<String, Macro> configuredMacros = Collections.emptyMap();
if (!configuredDefines.isEmpty()) {
LOG.debug("parsing configured defines");
configuredMacros = parseMacroDefinitions(configuredDefines);
}

// fill CxxPreprocessor.fixedMacros
// getMacros() returns CxxPreprocessor.fixedMacros
if (getMacros() != fixedMacros) {
throw new IllegalStateException("expected fixedMacros as active macros map");
}
try {
getMacros().setHighPrio(true);

// parse the configured defines and store into the macro library
for (String define : conf.getDefines()) {
LOG.debug("parsing external macro: '{}'", define);
if (!"".equals(define)) {
Macro macro = parseMacroDefinition("#define " + define);
LOG.debug("storing external macro: '{}'", macro);
getMacros().put(macro.name, macro);
}
}

// set standard macros
getMacros().putAll(Macro.STANDARD_MACROS);

// parse the configured force includes and store into the macro library
for (String include : conf.getForceIncludeFiles()) {
LOG.debug("parsing force include: '{}'", include);
if (!"".equals(include)) {
parseIncludeLine("#include \"" + include + "\"", "sonar." + this.language.getPropertiesKey()
+ ".forceIncludes", conf.getEncoding());
}
}
getMacros().putAll(configuredMacros);
parseForcedIncludes();
ivangalkin marked this conversation as resolved.
Show resolved Hide resolved
} finally {
getMacros().setHighPrio(false);
ctorInProgress = false;
}

// fill CxxPreprocessor.predefinedUnitMacros if relevant
final CxxCompilationUnitSettings globalCUSettings = conf.getGlobalCompilationUnitSettings();
if (!conf.getCompilationUnitSourceFiles().isEmpty() || (globalCUSettings != null)) {
unitMacros = new MapChain<>();
if (getMacros() != unitMacros) {
throw new IllegalStateException("expected unitMacros as active macros map");
}
try {
getMacros().setHighPrio(true);
getMacros().putAll(Macro.UNIT_MACROS);
getMacros().putAll(configuredMacros);
parseForcedIncludes();
predefinedUnitMacros = new HashMap<>(unitMacros.getHighPrioMap());
} finally {
unitMacros = null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

@ivangalkin is here getMacros().setHighPrio(false) missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, in this case we don't have to disable the high-prio mode

  1. unitMacros = new MapChain<>(); -> make getMacro() return unitMacros
  2. fill the map getMacro() directly and (more important) indirectly through parseForcedIncludes()
  3. all precompiled macros for compile units are now in unitMacros.getHighPrioMap(); we copy them into predefinedUnitMacros
  4. unitMacros = null; -> destroy the unitMacros; they won't be filled from constructor anymore, no need to toggle the high-prio mode

}
} else {
predefinedUnitMacros = Collections.emptyMap();
}

// fill CxxPreprocessor.globalCUDefines if relevant
if (globalCUSettings != null) {
LOG.debug("parsing global compilation unit defines");
globalCUDefines = parseMacroDefinitions(globalCUSettings.getDefines());
} else {
globalCUDefines = Collections.emptyMap();
}

ctorInProgress = false;
}

public static void finalReport() {
Expand Down Expand Up @@ -457,14 +525,14 @@ public void init() {
Objects.requireNonNull(context.getFile(), "SquidAstVisitorContext::getFile() must be non-null");
compilationUnitSettings = conf.getCompilationUnitSettings(currentContextFile.getAbsolutePath());

boolean useGlobalCUSettings = false;

if (compilationUnitSettings != null) {
LOG.debug("compilation unit settings for: '{}'", currentContextFile);
} else {
} else if (conf.getGlobalCompilationUnitSettings() != null) {
compilationUnitSettings = conf.getGlobalCompilationUnitSettings();

if (compilationUnitSettings != null) {
LOG.debug("global compilation unit settings for: '{}'", currentContextFile);
}
useGlobalCUSettings = true;
LOG.debug("global compilation unit settings for: '{}'", currentContextFile);
}

if (compilationUnitSettings != null) {
Expand All @@ -478,54 +546,32 @@ public void init() {
// Treat all global defines as high prio
getMacros().setHighPrio(true);

// parse the configured defines and store into the macro library
for (String define : conf.getDefines()) {
LOG.debug("parsing external macro to unit: '{}'", define);
if (!"".equals(define)) {
Macro macro = parseMacroDefinition("#define " + define);
if (macro != null) {
LOG.debug("storing external macro to unit: '{}'", macro);
getMacros().put(macro.name, macro);
}
}
}

// set standard macros
// using smaller set of defines as rest is provides by compilation unit settings
getMacros().putAll(Macro.UNIT_MACROS);

// parse the configured force includes and store into the macro library
for (String include : conf.getForceIncludeFiles()) {
LOG.debug("parsing force include to unit: '{}'", include);
if (!"".equals(include)) {
// TODO -> this needs to come from language
parseIncludeLine("#include \"" + include + "\"", "sonar.cxx.forceIncludes", conf.getEncoding());
}
}
// add macros which are predefined for each compilation unit
getMacros().putAll(predefinedUnitMacros);

// rest of defines comes from compilation unit settings
for (Map.Entry<String, String> entry : compilationUnitSettings.getDefines().entrySet()) {
final String name = entry.getKey();
final String body = entry.getValue();
getMacros().put(name, new Macro(name, body));
if (useGlobalCUSettings) {
getMacros().putAll(globalCUDefines);
} else {
getMacros().putAll(parseMacroDefinitions(compilationUnitSettings.getDefines()));
}
} finally {
getMacros().setHighPrio(false);
}

if (getMacro(CPLUSPLUS) == null) {
if (getMacro(Macro.CPLUSPLUS) == null) {
//Create macros to replace C++ keywords when parsing C files
getMacros().putAll(Macro.COMPATIBILITY_MACROS);
}
} else {
// Use global settings
LOG.debug("global settings for: '{}'", currentContextFile);
if (isCFile(currentContextFile.getAbsolutePath())) {
if (isCFile(currentContextFile.getName())) {
//Create macros to replace C++ keywords when parsing C files
getMacros().putAll(Macro.COMPATIBILITY_MACROS);
fixedMacros.disable(CPLUSPLUS);
getMacros().disable(Macro.CPLUSPLUS);
Copy link
Collaborator

Choose a reason for hiding this comment

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

think there are more macros different between C and C++?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the map Macro.STANDARD_MACROS_IMPL contains only two C++ specific macros:

  • __cplusplus
  • __has_include (since C++17)

so theoretically we have to disable __has_include to. BUT

  1. I tested GCC in the ANSI-C mode and it still provides this define (https://coliru.stacked-crooked.com/a/30a53f2c2a9a323d)
  2. Our implementation of __has_include is hard-wired in the grammar and the expansion of __has_include() is not restricted to C++. So even if #ifdef __has_include evaluates to 0, we still can't prevent __has_include(...) from evaluation.
  3. This C vs C++ was not scope of my change, so please let's consider to change it in other PR

} else {
fixedMacros.enable(CPLUSPLUS);
getMacros().enable(Macro.CPLUSPLUS);
}
}
}
Expand Down Expand Up @@ -710,6 +756,21 @@ private int expandFunctionLikeMacro(String macroName, List<Token> restTokens, Li
return tokensConsumedMatchingArgs;
}

/**
* Parse the configured forced includes and store into the macro library.
* Current macro library depends on the return value of
* CxxPreprocessor#getMacros()
*/
private void parseForcedIncludes() {
for (String include : conf.getForceIncludeFiles()) {
if (!include.isEmpty()) {
LOG.debug("parsing force include: '{}'", include);
parseIncludeLine("#include \"" + include + "\"", "sonar." + this.language.getPropertiesKey() + ".forceIncludes",
conf.getEncoding());
}
}
}

private List<Token> expandMacro(String macroName, String macroExpression) {
// C++ standard 16.3.4/2 Macro Replacement - Rescanning and further replacement
List<Token> tokens = null;
Expand Down Expand Up @@ -855,6 +916,42 @@ private Macro parseMacroDefinition(String macroDef) {
.getFirstDescendant(CppGrammar.defineLine));
}

/**
* Parse defines spited into key-value format
* (sonar.cxx.jsonCompilationDatabase)
*/
private Map<String, Macro> parseMacroDefinitions(Map<String, String> defines) {
final List<String> margedDefines = defines.entrySet().stream().map(e -> e.getKey() + " " + e.getValue())
.collect(Collectors.toList());
return parseMacroDefinitions(margedDefines);
}

/**
* Parse defines, which are merged into one string (see sonar.cxx.defines)
*/
private Map<String, Macro> parseMacroDefinitions(List<String> defines) {
final Map<String, Macro> result = new HashMap<>();

for (String define : defines) {
if (define.isEmpty()) {
continue;
}

final String defineString = "#define " + define;

LOG.debug("parsing external macro: '{}'", defineString);
final Macro macro = parseMacroDefinition(defineString);

if (macro != null) {
if (LOG.isDebugEnabled()) {
LOG.debug("storing external macro: '{}'", macro);
}
result.put(macro.name, macro);
}
}
return result;
}

private File findIncludedFile(AstNode ast, Token token, String currFileName) {
String includedFileName = null;
boolean quoted = false;
Expand Down
6 changes: 4 additions & 2 deletions cxx-squid/src/main/java/org/sonar/cxx/preprocessor/Macro.java
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public Macro(String name, @Nullable List<Token> params, @Nullable List<Token> bo
* @param name
* @param body
*/
public Macro(String name, String body) {
private Macro(String name, String body) {
this.name = name;
this.params = null;
this.body = Collections.singletonList(Token.builder()
Expand Down Expand Up @@ -96,6 +96,8 @@ private static void add(Map<String, Macro> map, String name, String body) {
map.put(name, new Macro(name, body));
}

public static final String CPLUSPLUS = "__cplusplus";

/**
* This is a collection of standard macros according to
* http://gcc.gnu.org/onlinedocs/cpp/Standard-Predefined-Macros.html
Expand All @@ -111,7 +113,7 @@ private static void add(Map<String, Macro> map, String name, String body) {
add(STANDARD_MACROS_IMPL, "__TIME__", "\"??:??:??\"");
add(STANDARD_MACROS_IMPL, "__STDC__", "1");
add(STANDARD_MACROS_IMPL, "__STDC_HOSTED__", "1");
add(STANDARD_MACROS_IMPL, "__cplusplus", "201103L");
add(STANDARD_MACROS_IMPL, CPLUSPLUS, "201103L");
// __has_include support (C++17)
add(STANDARD_MACROS_IMPL, "__has_include", "1");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
*/
package org.sonar.cxx.preprocessor;

import java.util.Collections;
import java.util.HashMap;
import java.util.Map;

Expand Down Expand Up @@ -120,4 +121,8 @@ private void move(K key, Map<K, V> from, Map<K, V> to) {
}
}

public Map<K, V> getHighPrioMap() {
return Collections.unmodifiableMap(highPrioMap);
}

}
Loading